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

 
 

You are not allowed to post comments on this tracker with your current authentication level.

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

Submitter:  David Lawrence Ramsey <dolorous>
Submitted:  Sat 05 Aug 2017 09:02:22 PM UTC
   
 
Severity:  3 - Normal Status:  Fixed
Assigned to:  bens Open/Closed:  Closed

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>
Group administrator
Thu 17 Aug 2017 07:07:56 PM UTC, comment #9: 

Thanks for the patch.  Pushed as e09dbf18.

Benno Schulenberg <bens>
Group administrator
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>
Group Member
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>
Group administrator
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>
Group Member
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>
Group administrator
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>
Group Member
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>
Group Member
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>
Group Member
Sat 05 Aug 2017 09:06:20 PM UTC, comment #1: 

The attached patch fixes this.

(file #41428)

David Lawrence Ramsey <dolorous>
Group Member
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>
Group Member

 

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

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bens (Updated the item)
  • -email is unavailable- added by dolorous (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-08-22 bens Open/ClosedOpen Closed
    2017-08-17 dolorous Attached File- Added 0001-display-always-display-whitespace-characters-for-tab.patch, #41552
        Attached File- Added nulltest.txt, #41553
    2017-08-14 bens StatusIn Progress Fixed
    2017-08-14 dolorous Attached File- Added tabtests.zip, #41520
    2017-08-14 bens Attached File- Added suppress-showing-partial-tabs-at-leftedge.patch, #41512
        StatusNone In Progress
        Assigned toNone bens
    2017-08-05 dolorous Attached File- Added 0001-display-don-t-show-visible-characters-for-softwrappe.patch, #41428

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code