bugGNU nano - Bugs: bug #51669, whitespace display mode shows two...

 
 

bug #51669: whitespace display mode shows two tab characters for tabs split across rows

Submitted by:  David Lawrence Ramsey <dolorous>
Submitted on:  Sat 05 Aug 2017 09:02:22 PM UTC  
 
Severity: 3 - NormalStatus: Fixed
Assigned to: Benno Schulenberg <bens>Open/Closed: Closed

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 22 Aug 2017 02:52:58 PM UTC, comment #10:

The problem was unreleased, thus closing this.

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Thu 17 Aug 2017 07:07:56 PM UTC, comment #9:

Thanks for the patch. Pushed as e09dbf18.

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Thu 17 Aug 2017 07:04:58 AM UTC, comment #8:

Thank you. However, there's one case I just found that your fix doesn't cover. I originally found it while adding tabs to the nano binary, but I've come up with a simpler case.

1. In an 80x24 terminal, with the attached text file, run:

./nano --ignore --softwrap nulltest.txt

2. Press Meta-P to toggle whitespace display mode on.

3. Press Down twice to put the cursor at the beginning of the third row, and then press Tab. No whitespace character for tab will be displayed there, even though it should be, since the tab starts at the beginning of the row.

The problem is this: due to all the two-column characters on the previous rows, the beginning column of the chunk is not an exact multiple of the tab size, which would normally mean that it shouldn't get a whitespace character. However, the tab starts at the beginning of the row, which means that it should get a whitespace character in spite of the previous case; your logic misses that one corner case.

The attached patch fixes this by checking for the additional condition of start_col being equal to column; this will catch tabs at the beginning of the row regardless. It passes all my previous testing as well.

(file #41552, file #41553)

David Lawrence Ramsey <dolorous>
Project Administrator
Mon 14 Aug 2017 07:36:10 PM UTC, comment #7:

Wow... extensive testing. I just fiddled a bit with the terminal width and thought it looked okay. :) And did this quick test both with and without --atblanks.

Fixed in git, 9f6c3a62.

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Mon 14 Aug 2017 06:03:39 PM UTC, comment #6:

After some testing, your patch does seem to cover all the cases I've tried, and it doesn't seem to be overlooking anything so far. Thanks.

(For the record, those cases are: with all of the files in the attached zip file in an 80-column terminal with a tab size of 8, with all of those files in an 80-column terminal with a tab size of 7, with all of those files in a 79-column terminal with a tab size of 8, with all of those files in a 79-column terminal with a tab size of 7, with all of those files in a 60-column terminal with a tab size of 8, and with all of those files in a 60-column terminal with a tab size of 7.)

(file #41520)

David Lawrence Ramsey <dolorous>
Project Administrator
Mon 14 Aug 2017 09:37:43 AM UTC, comment #5:

Attached is a simplified patch that also handles the case from comment #4. Is there anything I'm overlooking?

(file #41512)

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Sun 13 Aug 2017 04:28:55 AM UTC, comment #4:

Specifically, the problem the patch doesn't fix is reproduced as follows (from Benno Schulenberg, bug #51621):

"To reproduce, run:

src/nano --ignore --nowrap --softwrap --atblanks doc/faq.html

and type M-+ until the second chunk of a softwrapped line is on the top row. Then type <Tab>... Then type M-P to switch on visible whitespace."

The tab in question will be one column wide, and start at the beginning of the chunk, but will not have its whitespace character displayed after following these instructions, when, in that case, it should.

David Lawrence Ramsey <dolorous>
Project Administrator
Sun 13 Aug 2017 03:44:47 AM UTC, comment #3:

Thinking about it more, maybe the entire approach is wrong, and the changes to fix this need to be made in edit_draw() or something along those lines?

David Lawrence Ramsey <dolorous>
Project Administrator
Sun 13 Aug 2017 03:27:28 AM UTC, comment #2:

Actually, after more testing, it improperly blocks whitespace display of some tabs that start at the middle of a chunk, if the chunk is broken early due to atblanks softwrapping. No fix for this yet.

David Lawrence Ramsey <dolorous>
Project Administrator
Sat 05 Aug 2017 09:06:20 PM UTC, comment #1:

The attached patch fixes this.

(file #41428)

David Lawrence Ramsey <dolorous>
Project Administrator
Sat 05 Aug 2017 09:02:22 PM UTC, original submission:

Currently, in git 94b484e, if softwrap mode and whitespace display mode are both on, and a file contains enough tabs to wrap from one row onto the next, whitespace display mode will display a tab character both at the beginning of the tab, and at where the tab begins on the next row.

This is only currently visible with git if the patch in bug #51621 is applied so that tabs aren't broken too early. It also occurs with softwrap mode before the variable-width overhaul (in version 2.8.0), and before the softwrap overhaul (in version 2.7.5).

David Lawrence Ramsey <dolorous>
Project Administrator

 

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

Attach File(s):
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by bens (Updated the item)
  • -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 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 22 Aug 2017 02:52:58 PM UTCbensOpen/ClosedOpen=>Closed
    Thu 17 Aug 2017 07:04:58 AM UTCdolorousAttached File-=>Added 0001-display-always-display-whitespace-characters-for-tab.patch, #41552
      Attached File-=>Added nulltest.txt, #41553
    Mon 14 Aug 2017 07:36:10 PM UTCbensStatusIn Progress=>Fixed
    Mon 14 Aug 2017 06:03:39 PM UTCdolorousAttached File-=>Added tabtests.zip, #41520
    Mon 14 Aug 2017 09:37:43 AM UTCbensAttached File-=>Added suppress-showing-partial-tabs-at-leftedge.patch, #41512
      StatusNone=>In Progress
      Assigned toNone=>bens
    Sat 05 Aug 2017 09:06:20 PM UTCdolorousAttached File-=>Added 0001-display-don-t-show-visible-characters-for-softwrappe.patch, #41428

    Back to the top


    Powered by Savane 3.1-cleanup1