Adjust GetBackwardWordAmount and GetForwardWordAmount to take symbols into account #6496
Conversation
|
Seems logical. |
|
When selecting forward words, it should also select the symbol (using macOS as example here, don't have Windows to test with atm): Screen.Recording.2025-01-21.at.14.04.09.movScreen.Recording.2025-01-21.at.14.04.31.mov |
| // Skip one character if it is a symbol to avoid cases like "author=<cursor>" requiring two taps | ||
| if (isCharacterSymbol(text[searchPrev])) | ||
| searchPrev--; | ||
|
|
||
| if (searchPrev <= 0) return -selectionEnd; | ||
|
|
||
| Func<char, bool> searchFunc = isCharacterSymbol(text[searchPrev]) ? isCharacterSymbol : isCharacterAlphanumeric; | ||
| while (searchPrev > 0 && searchFunc(text[searchPrev])) | ||
| searchPrev--; |
There was a problem hiding this comment.
This function in general is quite hard to understand and prove for correctness, imo. I wonder if all of these can be combined into the one while loop? Maybe including the whitespace case from above?
Here's another weird case (multiple symbols before alphanumeric):
Screen.Recording.2025-01-21.at.14.11.29.mov
Screen.Recording.2025-01-21.at.14.11.44.mov
There was a problem hiding this comment.
I combined it all under a single loop and tried to make it easier to understand. Please let me know if it is alright now
When making this PR I used Visual Studio Code as a reference, sadly it looks like the way this is implemented in apps is inconsistent. |
I'd just choose one and go with it. Probably matching smoogi's proposal is fine until someone has a problem with it. It's not important in the scheme of things. |
|
Ah jeez, I didn't realize everything handles it differently. I was testing Firefox, but it also handles it differently compared to TextEdit/Safari... As long as it's consistent is fine then, I think the examples I pointed out are still showing inconsistency. |
|
Alright, I have updated the logic to match your examples above. Thank you for your feedback! |
Fixes #6483
osu.Framework.Tests_4dKSD5KYp0.mp4