fix(auto_pairs): fix auto pairs with crlf

[dead10ck]

Auto pairs were resulting in incorrect ranges in the selections when the line terminators are CRLF (i.e. Windows). It turns out this is because when we were checking if the selection was a single-width cursor, it incorrectly assumed that this would be a single char. This is not the case, as a cursor can cover a multi-code point grapheme. Therefore, we must instead explicitly work with and check graphemes to determine if the cursor should move or extend the selection.

Fixes #1436

[dead10ck]

@CptPotato would you please be able to test this locally on your Windows machine and verify the fix?

[archseer]

Are these two imports still necessary?

[dead10ck]

Yeah, they were necessary for seeing logs with unit tests. Though it might be worthwhile to leave this out until we set up common unit testing code for things like this.

[archseer]

Let's remove this for now, I personally either use a debugger or switch to println!() when debugging then remove it before commiting

[CptPotato]

@CptPotato would you please be able to test this locally on your Windows machine and verify the fix?

I've only done a quick test but it seems to work for me now 👍

[archseer]

Looks good otherwise, I've been trying to simplify this but haven't found a way yet. I think it might make things simpler if get_next_range is implemented per operation (open/close) since close always takes the if len_inserted == 0 path.

[archseer]

Let's remove this for now, I personally either use a debugger or switch to println!() when debugging then remove it before commiting

[dead10ck]

Looks good otherwise, I've been trying to simplify this but haven't found a way yet.

Yeah, the function is bigger than I would like, but I also can't think of a good way to simplify it.

I think it might make things simpler if get_next_range is implemented per operation (open/close) since close always takes the if len_inserted == 0 path.

This is actually not true, because you can be inserting a closing character by itself, in which case it must be 1 with the current code, and shares logic with the open path.

And after we have more flexible configs a la #1418, it could theoretically be longer lengths from international keyboards, in which case it shares even more logic.

Let's remove this for now, I personally either use a debugger or switch to println!() when debugging then remove it before commiting

Ok, done. I had been using println!, but I tend to forget it's there when I go to make a debug build to test out changes, and then the renderer completely barfs and makes a bunch of garbled text because the debug output is printing to stdout and much hilarity ensues.

But yeah, this should be something in the testing harness as part of #115 instead of scattering logging code across the code base in random modules.