Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Line ending handling with sort functions #26

Closed
Ste1io opened this issue Jan 9, 2023 · 11 comments
Closed

Line ending handling with sort functions #26

Ste1io opened this issue Jan 9, 2023 · 11 comments

Comments

@Ste1io
Copy link

Ste1io commented Jan 9, 2023

When inverting a selection of lines that that were selected by line (vs by character), the final line break at the end of the selection is included in the sorting, resulting in an unintended line break at the top of the selection, with the first unselected line following the sorted region being concatenated onto the last sorted line. Sorting options exhibit the same behavior, but without the blank line at the top.

I know that was a mouthful, so hopefully hopefully the gifs below make more sense. EOL sequence for the file is CRLF.

How to repro the issue (shows both sorting and invert behavior):

Code_NhOryjIUvU

Expected behavior would be identical to when selecting by character, as supposed to by line:

Code_f4YqGZ90UY

@carlocardella
Copy link
Owner

Thanks for reporting this, the gifs do help.
I have an idea on the cause, I'll see if I can fix it tomorrow

@Ste1io
Copy link
Author

Ste1io commented Jan 9, 2023

Checking if the first or last line's selection is completely empty except an EOL sequence, and both ignoring it and adjusting the selected line count by -1, would probably fix it.

The sort options already check the former, they just need the line count decremented when that is the case.

@Ste1io
Copy link
Author

Ste1io commented Jan 9, 2023

Just a quick observation for what it's worth:

Single-line selections, and multi-line selections made by dragging up (resulting in the EOL capture at the top, instead of the bottom) emit the same behavior. (Technically, a whole-line selection made on a single line would count as two lines anyways even if perceived as a single line, but worth noting nonetheless.)

Code_HTxxLdWLhh

@carlocardella
Copy link
Owner

tt sort selection

@carlocardella
Copy link
Owner

Released with 2.10.2 (pre-release version), it should be available in the Marketplace shortly

@carlocardella
Copy link
Owner

Just a quick observation for what it's worth:

Single-line selections, and multi-line selections made by dragging up (resulting in the EOL capture at the top, instead of the bottom) emit the same behavior. (Technically, a whole-line selection made on a single line would count as two lines anyways even if perceived as a single line, but worth noting nonetheless.)

e906199

@Ste1io
Copy link
Author

Ste1io commented Jan 10, 2023

Testing prerelease v2.10.3:

Line-selections are still including the orphan EOL in the sort, producing unexpected results. Additionally, the entire selection region is now shifted by one line. Note that "Line-selections" are referring to a full-line selection, made by clicking on the line number to the left. Notice how moving lines up and down using Alt with an up/down arrow exclude the line where the cursor appears below the selection. The sort option should do likewise.

Line selections made from top down:

Line selections made from bottom up:

@carlocardella
Copy link
Owner

Ok, for the first gif the problem is the cursor at the beginning on the new file (but without characters selected): the current logic uses the cursor's position to determine if the line should be sorted or not, regardless of an actual selection. I'll change that.

The secont gif is more surprising, line 13 (zz1) should not be touched at all... 🤔

@Ste1io
Copy link
Author

Ste1io commented Jan 10, 2023

The cursor position is important for placing it correctly after the sort operation, but it's of no relevancy to what gets sorted. In the first gif, it has to go on the next line since the line-selections include the EOL in the last line (seen by the extra non-existent space at the end of each highlighted line). Doing a normal char selection (click-drag the actual text, not the line numbers) will omit the final line's EOL in the selection because your selection area (which the cursor follows) stops on that last line (before EOL) not below it (after EOL).

Note that the cursor position is different after the sort, in the first gif, and in the second gif it is correct but the last line no longer shows the EOL as part of the selection.

Code_vdeyaFt1a1

carlocardella added a commit that referenced this issue Jan 12, 2023
carlocardella added a commit that referenced this issue Jan 12, 2023
@carlocardella
Copy link
Owner

586343f

Pre-release 2.10.4 should be available in the Marketplace shortly

@Ste1io
Copy link
Author

Ste1io commented Jan 12, 2023

💯 works perfectly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants