bugGNU nano - Bugs: bug #51671, --softwrap --atblanks mishandles...

 
 

bug #51671: --softwrap --atblanks mishandles multicolumn Unicode spaces

Submitted by:  David Lawrence Ramsey <dolorous>
Submitted on:  Sat 05 Aug 2017 10:53:55 PM UTC  
 
Severity: 2 - MinorStatus: Fixed
Assigned to: David Lawrence Ramsey <dolorous>Open/Closed: Open

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Tue 26 Sep 2017 07:05:48 PM UTC, comment #21:

Thanks for the patch. Pushed to git, 504a9725.

The doubt about char_len is "solved" by my reshuffling things a bit in a subsequent commit -- char_len might already be set to the right value, but we have to do the parse_mbchar() anyway (to get the column after the blank), so we might just as well assign the result to char_len, and be certain that the value is correct.

Benno Schulenberg <bens>
Project Administrator
Sun 24 Sep 2017 07:55:12 PM UTC, comment #20:

Thank you, and good to know; your reply seems to have put you in the carbon-copy list in this case.

As far as I can tell, this should be the final version of this patch, barring your commentary.

(Well... in the patch, I've wondered whether char_len needs to be set when calling parse_mbchar() and incrementing the text pointer, since it should already be set to the right value if the whitespace at the end overshoots the screen edge, but not setting it before incrementing the pointer might lead to another manifestation of bug #51770, so for now I'm being paranoid and setting it.)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Sun 24 Sep 2017 07:28:35 PM UTC, comment #19:

Oh! I hadn't seen that you have updated this patch several times. When... you feel that you've reached the final version, don't hesitate to put me (bens) in the Carbon-Copy list. (Only when a new bug or patch is posted, I get an email, but afterward not until I have made a comment on the bug or patch.)

Will look into this some day this coming week.

Benno Schulenberg <bens>
Project Administrator
Sun 17 Sep 2017 07:21:49 PM UTC, comment #18:

Attached one last version. The only change relative to the last one is to remove a comment that merely re-describes what the code does. Now, these changes make the patched code take up one less line than the original code.

(file #41821)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Sat 16 Sep 2017 09:50:31 PM UTC, comment #17:

Attached yet another new version that simplifies things again.

prev_column doesn't need to be set to lastblank_column at all in the atblanks handling. If a multi-column Unicode space overshoots the screen, it's already set to the right column, and if a tab overshoots the screen, the tab-handling logic sets it to the right column.

(file #41816)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Fri 15 Sep 2017 05:45:54 PM UTC, comment #16:

Attached one last new version that cosmetically reduces the number of lines added (one less blank line, and a shorter added comment explaining the fallthrough condition).

(file #41811)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Tue 12 Sep 2017 03:21:26 PM UTC, comment #15:

Attached one more new version. This reshuffles things to be in the same order as the rest of get_softwrap_breakpoint(), and eliminates a temporary variable in the process.

(file #41787)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Sat 26 Aug 2017 10:46:44 AM UTC, comment #14:

...and fix an ambiguity in the comment dealing with reversed logic: "there" does not mean the edge of the screen in that case.

(file #41667)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Sat 26 Aug 2017 10:23:17 AM UTC, comment #13:

Attached one last new version. The comment changes are now reduced to the absolutely necessary ones: one to indicate reversed logic relative to current git, and one to indicate a fallthrough condition that current git doesn't have.

(file #41666)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Thu 24 Aug 2017 11:03:38 PM UTC, comment #12:

Attached new version fixes a potential lockup on a two-column screen with --atblanks and Unicode 003000. (It also shortens the commit message a bit.)

In this case, after moving forward to after the blank, the column can be equal to the edge of the screen, so we try to move before it, and the character just before it is the same character we started on. This leads to an infinite loop trying to find a breakpoint.

So, if the column after the blank is equal to the edge of the screen, we now return it instead. This avoids the infinite loop, and also matches the behavior in non-atblanks mode: end_of_line isn't set, so if the Unicode 003000 character is at the end of the line, nano will push the newline to the next line as expected.

(file #41648)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Thu 24 Aug 2017 09:58:01 PM UTC, comment #11:

...and simplified it again. The comments are now shorter, and the text pointer is now incremented in atblanks mode only if necessary.

(file #41647)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Thu 24 Aug 2017 09:26:23 PM UTC, comment #10:

Attached new version that's a bit simpler (it adds one less line).

(file #41646)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Sun 20 Aug 2017 04:54:38 PM UTC, comment #9:

Attached new version with improved comments (they take up the same number of lines as before).

(file #41591)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Wed 16 Aug 2017 06:47:54 PM UTC, comment #8:

Attached new, even simpler version that has less duplicate code. (It does add a few comments to clarify what's going on, however.)

(file #41543)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Tue 15 Aug 2017 09:31:55 PM UTC, comment #7:

Forget that one; it breaks handling of tabs if, say, you opened the nano binary with itself. This new version fixes the problem, and uses even less code in the process. Sorry for the mess.

(file #41536)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Tue 15 Aug 2017 09:25:12 PM UTC, comment #6:

Attached new version: it's reworked a bit to take up one less line.

(file #41535)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Tue 15 Aug 2017 06:03:36 PM UTC, comment #5:

And now that the aforementioned bug is fixed and its relevant code is applied, here's a proper standalone version of this patch, against git 8b86af3.

(file #41528)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Sun 13 Aug 2017 04:21:01 PM UTC, comment #4:

Updated again, to account for the new patch at bug #51621.

(file #41506)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Sun 13 Aug 2017 03:32:23 AM UTC, comment #3:

Updated fix attached, that doesn't use incorrect logic when adding tabs to the beginning of a chunk after the first one in the line.

(file #41499)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Sun 06 Aug 2017 03:35:00 PM UTC, comment #2:

Addendum: the fix is attached, although it requires the patch at bug #51621 to be applied first.

(file #41438)

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Sat 05 Aug 2017 11:01:14 PM UTC, comment #1:

For the record, the fix for this can be found here:

https://savannah.gnu.org/bugs/?51621

since it conflicts with the fix that bug needs, and thus depends on it.

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
Sat 05 Aug 2017 10:53:55 PM UTC, original submission:

Currently, in git 94b484e, if nano is running with softwrap and atblanks, it should break before multi-column Unicode blanks if they would go over the edge of the screen, but it doesn't. This can be reproduced as follows:

1. Run nano in ax 80x24 terminal, with --ignore --softwrap --atblanks.

2. Press Meta-V 0 0 3 0 0 0 to insert Unicode character 003000 (Ideographic space), a two-column Unicode blank.

3. Press Left to move the cursor back, and then hold down the "a" key. Note that when the two-column blank after all the "a"s is at the edge of the screen, only half of it is displayed, when it should be moved to the next row in that case.

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by bens (Posted a comment)
  • -unavailable- added by dolorous (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 22 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 26 Sep 2017 07:05:48 PM UTCbensStatusIn Progress=>Fixed
      Summaryatblanks softwrap mishandles multicolumn Unicode blanks=>--softwrap --atblanks mishandles multicolumn Unicode spaces
    Sun 24 Sep 2017 07:28:35 PM UTCbensSeverity3 - Normal=>2 - Minor
      StatusNone=>In Progress
      Assigned toNone=>dolorous
    Sun 17 Sep 2017 07:21:49 PM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41821
    Sat 16 Sep 2017 09:50:31 PM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41816
    Fri 15 Sep 2017 05:45:54 PM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41811
    Tue 12 Sep 2017 03:21:26 PM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41787
    Sat 26 Aug 2017 10:46:44 AM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41667
    Sat 26 Aug 2017 10:23:17 AM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41666
    Thu 24 Aug 2017 11:03:38 PM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41648
    Thu 24 Aug 2017 09:58:01 PM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41647
    Thu 24 Aug 2017 09:26:23 PM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41646
    Sun 20 Aug 2017 04:54:38 PM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41591
    Wed 16 Aug 2017 06:47:54 PM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41543
    Tue 15 Aug 2017 09:31:55 PM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41536
    Tue 15 Aug 2017 09:25:12 PM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41535
    Tue 15 Aug 2017 06:03:36 PM UTCdolorousAttached File-=>Added 0001-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41528
    Sun 13 Aug 2017 04:21:01 PM UTCdolorousAttached File-=>Added 0002-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41506
    Sun 13 Aug 2017 03:32:23 AM UTCdolorousAttached File-=>Added 0002-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41499
    Sun 06 Aug 2017 03:35:00 PM UTCdolorousAttached File-=>Added 0002-softwrap-break-before-multi-column-Unicode-blanks-if.patch, #41438

    Back to the top


    Powered by Savane 3.1-cleanup1