jdtsmith / indent-bars

Fast, configurable indentation guide-bars for Emacs
GNU General Public License v3.0
272 stars 7 forks source link

Issue with evil next line #22

Closed walseb closed 8 months ago

walseb commented 9 months ago

Hello!

I have a strange issue when this mode is enabled, and I call next-line.

Try putting your cursor at the very end of foo in the code snippet below with this mode enabled, and run next-line, either interactively or via M-x.

                           foo

bar

For me, it jumps all the way down to bar. Can anyone else recreate it?

I'm running the latest version available on Melpa, and Emacs 29.

Thanks!

jdtsmith commented 9 months ago

I don't see this behavior in a python-ts-mode buffer. See if you can reproduce from emacs -q loading only indent-bars and the major mode of interest and let me know.

walseb commented 9 months ago

Hi!

I did some testing, and it appears to only be an issue when using evil-mode in the normal state. Here's how to reproduce it in emacs -q:

(require 'evil)

(evil-mode)

(require 'indent-bars)

(indent-bars-mode 1)

Then, insert the following into the scratch buffer, but any buffer should do.

                           foo

bar

Put your cursor at the end of foo, enter normal state, and press j. It should go all the way down to bar if indent-mode is enabled.

jdtsmith commented 9 months ago

This sounds like an "evil feature" of some kind to me (e.g. is j really just next-line?). I'm not an evil user and do not have it installed. Are there bars displayed on the blank lines? Try turning off indent-bars-display-on-blank-lines; does that fix it? If so, one thought is that evil does not like having newlines with display properties generally. You could test that by evaluating (dotimes (i 10) (insert (propertize "\n" 'display (format "test%d\n" i)))) in scratch and see whether the same navigation problems occur..

walseb commented 9 months ago

Hi!

j isn't bound to next-line, but interestingly, running next-line while in normal mode in that position causes the same issue to happen as pressing j and running the evil version of that command.

I tried setting indent-bars-display-on-blank-lines to nil and it fixed the problem! It would be nice to have them display on blank lines, though.

Also, it's worth noting that this issue only occurs when moving to the next line, not the previous line.

Running (dotimes (i 10) (insert (propertize "\n" 'display (format "test%d\n" i)))) with indent-bars enabled and disabled inserts whitespace that behaves like any other whitespace and doesn't have the same problem.

Since this issue doesn't occur in vanilla Emacs, I think this is an issue with evil. I will link the incompatibility on their bug-tracker.

jdtsmith commented 9 months ago

One theory is that for some reason these contiguous newlines with the same display string (value) are being merged together, or more likely "helpfully" skipped by an evil configuration/addition/patch to next-line (is there any advice on that function? C-h f next-line).

See this discussion. This is the reason that indent-bars creates a brand new display string for each blank line. But despite the unique strings (with the same value), perhaps evil somehow merges these into one block for navigation? Can you navigate via forward-char normally through that block?

Go to the first string which starts the skipped block, and C-u C-x =. What is the display property? If you can do so, do this same thing via forward-char on the start of each line. Then try adding a differing amount of space on each blank line; does this change the behavior?

jdtsmith commented 9 months ago

@walseb can you try these tests?

walseb commented 9 months ago

Hi!

Sorry for the delay. I will reply to this soon.

walseb commented 9 months ago

Hello! I tried what the evil-mode maintainer suggested in vanilla Emacs, and got the same issue.

Running this code snippet without evil-mode causes the same issue to occur:

(let (line-move-visual) (line-move 1)) 

Can you navigate via forward-char normally through that block?

It appears to work fine, although it stops at newlines, I'm not sure if that's the default behavior or if it's a setting in evil-mode.

I will try your other suggestions soon.

jdtsmith commented 9 months ago

I looked into this, and it appears the culprit is at quite a low level in move-to-column (in C code). The distinct evil way of moving line activates an attempt to move to a temporary target column (the starting column; in your example, column 30). move-to-column apparently gets confused about newlines with a display property, and moves past the line end onto subsequent lines, which should never happen.

Here is a test that demonstrates this:

(progn
  (insert "\n>>>>\n" (propertize "\n" 'display "xxxxx\n") "SHOULD NOT MOVE HERE\n<<<<")
  (forward-line -2) ; at beginning of xxxxx\n line
  (move-to-column 10)) ; should not move past the line

If there is no display property (e.g. xxxxx\n) on the newline, point stays on the line, as it should. With a display property on the newline, point moves to the next line; apparently because move-to-column has forgotten its rule to go no further than the end of the line. Can you please submit this as an emacs bug and link here?

walseb commented 9 months ago

Thank you for tracking down the issue!

I will submit the issue soon.

jdtsmith commented 8 months ago

Did this get submitted as an issue? Can you post it here?

walseb commented 8 months ago

Hello!

Sorry, I forgot to send it.

I sent it just now, I will send a link to the email chain when the archives are updated tomorrow.

Thank you for your patience!

walseb commented 8 months ago

Hi!

I got a reply from Eli:

It's the intended behavior. It is the best Emacs can do given the limitations posed by such overlay strings, see the detailed explanation in bug#64988. My best recommendation is not to use overlay strings with embedded newlines, if accurate movement by columns is important.

Ref: https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-11/msg00217.html

So it won't be fixed upstream. I suppose I could fix the issue locally by patching evil-mode to not use line-move-visual.

Thoughts?

jdtsmith commented 8 months ago

Was afraid of that; it's not clear why display strings with new lines cannot be respected. You could patch evil mode to use a move-to-column that does so (check the newline position for a 'display, and limit the move to there).

Unfortunately in order to add bars to blank lines, there is nowhere other than the newline to put the display property. So not much can be done here.

jdtsmith commented 8 months ago

Not much I can do about this given the odd behavior of Emacs moving to column on lines with newlines in their display properties.

Options are to disable indent-bars-display-on-blank-lines, or work around this, for example by patching line-move-to-column, like so:

(advice-add 'line-move-to-column :around
        (defalias 'my/indent-bars-prevent-passing-newline
          (lambda (orig col &rest r)
        (if-let ((indent-bars-mode)
             (nlp (line-end-position))
             (dprop (get-text-property nlp 'display))
             ((seq-contains-p dprop ?\n))
             ((> col (- nlp (point)))))
            (goto-char nlp)
          (apply orig col r)))))
walseb commented 8 months ago

Thank you so much!

Maybe this should be linked in a notice to evil users on the readme.

jdtsmith commented 8 months ago

Did you try this and did it work?

walseb commented 8 months ago

I tried indent-bars-display-on-blank-lines and it solved the problem. I will try out the patch soon, it would be nice to not have to disable indent bars on blank lines.

jdtsmith commented 8 months ago

OK let me know, if it works for you I can link from the README. There's simply no way to display a bunch of bars on a fully blank line besides 'display property on the final \n.

walseb commented 8 months ago

Hello!

I tried that function just now and it works perfectly.

I haven't seen any negative side effects of advising it, I will report any here if I find them.

Thanks so much!

walseb commented 8 months ago

You could exchange the defalias to a defun with recent Emacs versions, by the way:

(advice-add 'line-move-to-column :around
            (defun my/indent-bars-prevent-passing-newline (orig col &rest r)
          (if-let ((indent-bars-mode)
               (nlp (line-end-position))
               (dprop (get-text-property nlp 'display))
               ((seq-contains-p dprop ?\n))
               ((> col (- nlp (point)))))
              (goto-char nlp)
            (apply orig col r))))
jdtsmith commented 8 months ago

Didn't know that, thanks.