bugGNU nano - Bugs: bug #52258, with softwrap, combining...

 
 

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

bug #52258: with softwrap, combining characters at end of chunk are not displayed

Submitter:  Peter Passchier <pepa65>
Submitted:  Sat 21 Oct 2017 02:15:34 AM UTC
   
 
Severity:  3 - Normal Status:  Fixed
Assigned to:  dolorous Open/Closed:  Closed

Jump to the original submission

Sun 19 Nov 2017 08:26:23 AM UTC, comment #18: 

Released in 2.9.0.  Thanks for reporting.  And fixing.

Benno Schulenberg <bens>
Group administrator
Wed 25 Oct 2017 05:14:54 PM UTC, comment #17: 

The patch looks logical to me.  The only thing is: the mbwidth() function is somewhat costly, and it needs to be called only when column == beyond.  So... before pushing, I have swapped the two conditions.

Fixed in git, d344c3d0.  Thanks for the patch.  And thanks for reporting.

Benno Schulenberg <bens>
Group administrator
Tue 24 Oct 2017 03:38:28 PM UTC, comment #16: 

I've tried it with all the problematic lines from the file in comment #1, both with and without softwrap.  Adding text until the combining characters are at the edge of the screen works in softwrap mdoe, and scrolling forward and back works when not in softwrap mode.  So, in short, everything seems to work.

David Lawrence Ramsey <dolorous>
Group Member
Mon 23 Oct 2017 07:26:14 PM UTC, comment #15: 

Attached a patch that seems to fix it; I'm still testing it.

(file #42243)

David Lawrence Ramsey <dolorous>
Group Member
Mon 23 Oct 2017 06:13:00 PM UTC, comment #14: 

And after more testing, the problem is not limited to softwrap mode: any line run through display_string() that ends in combining characters has problems (although outside of softwrap mode, they're different problems).  For example, in an 80x24 terminal, and current git, run:

nano --ignore --softwrap --nowrap combining-chars2.txt

The line is exactly 80 columns wide, so fits exactly in one row of the terminal.  As we've seen, the combining character at the end of the line is not displayed.

Now, press Meta-$ to toggle softwrap mode off.  The final character of the line, plus its combining character, both disappear entirely.  Then press End, and you'll see that the character and its following combining character are still there, since they are then drawn before the edge of the screen.  Finally, press Backspace to delete the combining character, and press Home.  The final character of the line will then be displayed (minus the backspaced-over combining character, of course).

So it seems that the real problem is that display_string() has problems displaying combining characters at the final column of the string passed to it.  In short:

1. The original softwrap issue is triggered because softwrapping a line before the edge of the screen means that the final column of the string passed to it is the final column of the displayed line, so the problem always shows up if combining characters occur at the end of a softwrap chunk shorter than the screen width.

2. The new non-softwrap issue is triggered because, outside of softwrap mode, display_string() is generally called with a final column of editwincols, so the problem always shows up if combining characters occur at the final column at the edge of the screen.

(file #42242)

David Lawrence Ramsey <dolorous>
Group Member
Mon 23 Oct 2017 05:32:53 PM UTC, comment #13: 

And note that a chunk with missing combining characters due to this bug doesn't actually drop the characters (nano's navigation doesn't skip over them, so trying to move over them reveals them, and you can also tell by the column count that they're there); the characters are in the chunk, but they just aren't displayed.  I've updated the bug description to reflect this.

David Lawrence Ramsey <dolorous>
Group Member
Mon 23 Oct 2017 05:14:53 PM UTC, comment #12: 

And, for the record, nanocrap2.txt is the same file as the one attached to:

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

David Lawrence Ramsey <dolorous>
Group Member
Mon 23 Oct 2017 05:13:08 PM UTC, comment #11: 

Addendum: Actually, my patch does cause some breakage in certain cases.  With it, and current git (4d35835), run:

nano --ignore --nowrap nanocrap2.txt

Press End, and then press Home; the first character of the two-column "^@" will erroneously be on the next line.

So that patch won't work, but it still narrowed down the problem: the column range check in display_string() that uses beyond is cutting the line off too early, before the zero-width combining characters gets displayed.

David Lawrence Ramsey <dolorous>
Group Member
Mon 23 Oct 2017 05:04:27 PM UTC, comment #10: 

Looking into this, it isn't really the softwrap code per se causing this, it's the display_string() calls that the softwrap code is making to display the strings that's causing it.  Since display_string() takes a range of columns to display, and combining characters are zero columns wide, that's leading to their being cut off (and only on lines that take up one row or less).

The attached experimental patch seems to fix it.  Given the comments in display_string() regarding column and span, this shouldn't work, but it does in my preliminary testing so far, so maybe the comments are wrong somewhere?  (The fact that, according to comments, column is one-based while span is zero-based makes this rather confusing.  Or maybe at least one of those two assumptions is not actually true?)

(file #42238)

David Lawrence Ramsey <dolorous>
Group Member
Mon 23 Oct 2017 04:01:45 PM UTC, comment #9: 

Okay!  Can reproduce now.  Getting the author of the softwrap code into the loop... CC'ed.

Benno Schulenberg <bens>
Group administrator
Mon 23 Oct 2017 02:30:43 AM UTC, comment #8: 

It's a bug with softwrap. If I turn it off, all is normal.

Peter Passchier <pepa65>
Sun 22 Oct 2017 04:59:05 PM UTC, comment #7: 

So, if you take the 2.8.5 tarball and compile that, it renders the UTF testing text fine?  Do you use softwrapping?  And if you run the last released version (2.8.7) with --ignorercfiles, does it maybe render fine?  How wide are the terminals that you use?

[Trying to reproduce this bug, I've run into an absurd rendering bug on Linux Mint 18 Sarah (which is based on Ubuntu 16.04, Xenial, if I'm not mistaken).  Will report that shortly, although I don't think it's a nano bug.]

Benno Schulenberg <bens>
Group administrator
Sun 22 Oct 2017 02:43:01 PM UTC, comment #6: 

The v3.5.3 branch is working OK here.

I've never done git bisect, but to rebuild each time I did:
./autogen.sh
./configure
make
src/nano ~/UTF-8-demo.txt

And then marked the bisect as good or bad. The end result was:

Bisecting: 0 revisions left to test after this (roughly 0 steps)
[8490f4acab53ed38c3b083871654a073e293e02c] softwrap: make the changes to actually allow the chunk width to vary

Peter Passchier <pepa65>
Sun 22 Oct 2017 06:48:51 AM UTC, comment #5: 

If you do a 'git checkout v2.5.3' and build that, does that "suppress" some of the vectors and tone marks?

If yes, then it is a build problem.  If no, then please do a 'git bisect' to find the commit that broke this.

Benno Schulenberg <bens>
Group administrator
Sat 21 Oct 2017 02:44:03 PM UTC, comment #4: 

Well, that makes it non-reproducable I guess... Still wonder why that happens here only with nano.

I installed nano from the package manager:
 GNU nano, version 2.5.3
 (C) 1999..2016 Free Software Foundation, Inc.
 Email: nano@nano-editor.org    Web: http://www.nano-editor.org/
 Compiled options: --disable-libmagic --disable-wrapping-as-root --enable-utf8

And this version is fine. Is it something in the build somehow?

Peter Passchier <pepa65>
Sat 21 Oct 2017 09:36:51 AM UTC, comment #3: 

Yes, the vector sign is on the b like it is on the a in line 57, and the tone mark at the end of line 123 is the same as the mark in the middle of the first half of line 127, and the mark at the end of line 129 is the same as the one at the start of line 130.  They are all there.  Visible and deletable and restorable.

Benno Schulenberg <bens>
Group administrator
Sat 21 Oct 2017 08:44:26 AM UTC, comment #2: 

Hmm, then it sounds like I might have a local problem..? The vector sign on the b is missing in line 57, and the tone marks on the last phonemes of lines 123 and 129. Just to check: those are all present on your system?

I just pulled nano from the github master branch and did:
./configure
make
sudo make install

Terminals: Sakura, ROXTerm, MATE Terminal
Locale en_US.UTF-8
Distro: Ubuntu MATE 16.04.3

Peter Passchier <pepa65>
Sat 21 Oct 2017 08:19:13 AM UTC, comment #1: 

I don't see any differences between how nano renders those lines and how less and vim do.

Which terminal are you using?  Which locale?  What distro and version?

For me: xfce4-terminal 0.8.4 (Xfce 4.12), nl_NL.UTF-8, Xubuntu 17.04.

Benno Schulenberg <bens>
Group administrator
Sat 21 Oct 2017 02:15:34 AM UTC, original submission:  

Comparing nano's rendering with cat and vi of http://www.cl.cam.ac.uk/%7Emgk25/ucs/examples/UTF-8-demo.txt
exposed little rendering errors in lines 57, 123, 129.

 GNU nano from git, v2.8.7-56-gca8317e
 (C) 1999-2011, 2013-2017 Free Software Foundation, Inc.
 (C) 2014-2017 the contributors to nano
 Email: -email is unavailable- Web: https://nano-editor.org/
 Compiled options: --disable-libmagic --enable-utf8

Peter Passchier <pepa65>

 

(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 dolorous (Updated the item)
  • -email is unavailable- added by bens
  • -email is unavailable- added by bens (Posted a comment)
  • -email is unavailable- added by pepa65 (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 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-11-19 bens Open/ClosedOpen Closed
    2017-10-25 bens StatusConfirmed Fixed
        Assigned toNone dolorous
    2017-10-23 dolorous Attached File- Added 0001-display-don-t-cut-off-zero-width-characters-at-the-e.patch, #42243
    2017-10-23 dolorous Attached File- Added combining-chars2.txt, #42242
    2017-10-23 dolorous Summarywith softwrap, combining characters at end of chunk are dropped with softwrap, combining characters at end of chunk are not displayed
    2017-10-23 dolorous Attached File- Added off-by-one-display-string.patch, #42238
    2017-10-23 bens StatusNeed Info Confirmed
        SummaryUnicode rendering bugs with softwrap, combining characters at end of chunk are dropped
        Carbon-Copy- Added dolorous
    2017-10-21 bens StatusNone Need Info

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code