mawww / kakoune

mawww's experiment for a better code editor
http://kakoune.org
The Unlicense
10k stars 715 forks source link

Spurious whitespace at cursor position on blank line #1338

Open ndarilek opened 7 years ago

ndarilek commented 7 years ago

As a screen reader user, if I have a line like:

        console.log("testing")

My screen reader speaks: '8 spaces: console.log("testing")'

When I navigate past a blank line with Kakoune active, I hear something like "1 space". So if I type something like:

aThis is the first line.

This is the second.

then arrow up, I hear "1 space", then again, "This is the first line." There obviously is no space on the second line. When I arrowed through these with my browser, I heard "blank," then "aThis is the first line." (accounting for the command character to enter insert mode, of course.)

To be clear, these don't appear to be in the file itself, but I'm wondering if Kakoune is somehow using an invisible whitespace to place the cursor on a blank line such that it has space to occupy? As a visual user, you wouldn't see the space, but as a screen reader user it gets reported to me. I don't know a whole lot about how terminal output works.

For comparison, all Vim variants exhibit this same problem. Nano doesn't, and speaks "blank" if I arrow across a line with no content.

Not a major showstopper, but it does make blank lines slightly confusing, as I can't tell if the line is actually blank or has a single whitespace on it for some reason. :)

Thanks, happy to provide additional information if needed.

mawww commented 7 years ago

So, in Kakoune, we try to treat end of lines as regular characters as much as possible, so when you get on an empty line, the cursor is actually on the end-of-line character. Unfortunately, we cannot print an end of line character there, as it will not occupy the terminal cell, but just move the cursor, so we write a space character instead (the cursor is onto a character, not in between two).

Now, maybe there is a better character we can write there.

laelath commented 7 years ago

Making it clear that there's trailing whitespace probably isn't something the editor should do by default, if you are concerned about extra whitespace I would recommend using either addhl show_whitespaces which displays characters representing the whitespace, or addhl regex "[ \t]+$" 0:black,red which highlights trailing whitespace.

ndarilek commented 7 years ago

@mawww: Ah, suspected that might be the case. I poked around a bit in the code this morning before filing the issue but couldn't find where this was done. Mind pointing me to where this specific character is added? I can try a few others, see what the results are, and submit a PR if I find something that works.

In particular, maybe \r would be a good candidate. It's spurious, but more semantically correct than a space (I.e. it returns to the beginning of a blank line, so would be more of a no-op than space.) Since it isn't being written to the file, it wouldn't clash with Unix line endings. And since it's an EOL character, it probably won't be read by the screen reader in the way Space would. If there aren't objections to that, I can try adding it locally and test things out.

lenormf commented 7 years ago

Here is a patch that replaces those space characters with \r:

diff --git a/src/ncurses_ui.cc b/src/ncurses_ui.cc
index 9313e076..cfad0712 100644
--- a/src/ncurses_ui.cc
+++ b/src/ncurses_ui.cc
@@ -364,7 +364,7 @@ void NCursesUI::draw_line(NCursesWin* window, const DisplayLine& line,
             content.column_length() - 1 < remaining_columns)
         {
             add_str(window, content.substr(0, content.length()-1));
-            waddch(window, ' ');
+            waddch(window, '\r');
         }
         else
         {
ndarilek commented 7 years ago

Yes, this works. Thanks! Should I submit a PR?

lenormf commented 7 years ago

The issue with this patch is that now the cursor disappears when it's above a newline, and it used to be rendered as inverted video when going over the former space character. We need to think about this a bit more.

ndarilek commented 7 years ago

Hmm, there's another minor issue now that I'm doing more extensive testing. When I load actual code into this build, I'm getting lines that speak "118 spaces" and do, indeed, seem to have a large quantity of whitespace on them. Not sure if these match the width of my longest line or something.

These aren't as huge of a deal, since the likelihood that I have 118 spaces on a line is significantly less than me having a few that I accidentally missed, but it'd be great to fix all cases of this.

Thanks.

ndarilek commented 7 years ago

Actually, looks like the "118 spaces" is a tmux thing. Happens when I insert blank lines in a kak session running in a tmux pane. So not sure what can be done about that, though it doesn't happen in nano FWIW.

lenormf commented 4 years ago

Does the issue still occur?