fox0430 / moe

A command line based editor inspired by Vim. Written in Nim.
https://editor.moe
GNU General Public License v3.0
648 stars 34 forks source link

[Bug Report] Calling Suggestions on Paths with Typos can Crash on Opening Files with `:e` #1611

Closed kevinmatthes closed 1 year ago

kevinmatthes commented 1 year ago

@fox0430

This is a rather serious bug, in my opinion.

The best description is the set of instructions how to reproduce:

  1. Create a new directory somewhere, for example bug/ and go there.
  2. With bug/ as your working directory, create a new directory src/ as a sub-directory of bug/.
  3. Create two new files, say source1.nim and source2.nim in src/.
  4. With bug/ as your working directory, call moe src/source1.nim.
  5. With or without making changes to src/source1.nim, call :e src/soru and press the tab key once to try to get the suggestion for src/source2.nim. This should crash the editor. Please make source to intentionally type the name of the intended file wrong to simulate the typo src/soruce2.nim instead of the actual intention src/source2.nim (o and r are exchanged).

In a common development workflow, users often open already existing files to edit them. As typos may happen in the source code, typos can be entered unintentionally in the path description to open a file, as well. To reduce the possibility of spelling a path wrong, the suggestion feature of Ex Mode is quite useful. It is common to enter the first few characters to reduce the length of the suggestion list. If a rather simple typo breaks the session, the unsaved changes to the opened buffers will be lost. Thus, I consider this bug serious.

fox0430 commented 1 year ago

Stack trace

/home/fox/git/moe/src/moe.nim(58) moe
/home/fox/git/moe/src/moe.nim(54) main
/home/fox/git/moe/src/moepkg/mainloop.nim(342) editorMainLoop                                                                                                                          /home/fox/git/moe/src/moepkg/mainloop.nim(165) commandLineLoop
/home/fox/git/moe/src/moepkg/commandlineutils.nim(461) updateSuggestWindow                                                                                                                /home/fox/git/moe/src/moepkg/popupwindow.nim(33) writePopUpWindow
/home/fox/.choosenim/toolchains/nim-1.6.10/lib/system/fatal.nim(54) sysFatal
Error: unhandled exception: index 1 not in 0 .. 0 [IndexDefect]

Update

/home/fox/git/moe/src/moe.nim(58) moe
/home/fox/git/moe/src/moe.nim(54) main
/home/fox/git/moe/src/moepkg/mainloop.nim(342) editorMainLoop
/home/fox/git/moe/src/moepkg/mainloop.nim(165) commandLineLoop
/home/fox/git/moe/src/moepkg/commandlineutils.nim(469) updateSuggestWindow
/home/fox/git/moe/src/moepkg/popupwindow.nim(33) writePopUpWindow
/home/fox/.choosenim/toolchains/nim-1.6.10/lib/system/fatal.nim(54) sysFatal
Error: unhandled exception: index 1 not in 0 .. 0 [IndexDefect]
kevinmatthes commented 1 year ago

@fox0430

In my opinion, status.suggestionWindow.get in line 299 of src/moepkg/mainloop.nim calculates a wrong index for the suggestion window in case of a typo. Where is this getter defined?

fox0430 commented 1 year ago

@kevinmatthes

It's Option type.

https://github.com/fox0430/moe/blob/8804d7371b2456d1ff0795ba5ebbedc0f28ac583/src/moepkg/editorstatus.nim#L39

kevinmatthes commented 1 year ago

@fox0430

Okay, thank you. I did not mind the Option.

The bug should be somewhere in the search routines; the index breaking the session originates from line 157 in src/moepkg/mainloop.nim. I will try to locate the exact origin of the wrong calculation.

kevinmatthes commented 1 year ago

@fox0430

There might be a connection to the search history; line 159 of src/moepkg/editorstatus.nim.

The search history is loaded during the session start-up and then searched to retrieve the index for the suggestions which leads to an index value of 1 although there is no such file.

kevinmatthes commented 1 year ago

@fox0430

I am sorry, I looked up the index origin for the editor loop instead of the command line loop (and furthermore lost the trace which caused the suggestion with the search history). I will search again for the suggestion window of the command line, now.

kevinmatthes commented 1 year ago

@fox0430

Suggestion: src/moepkg/popupwindow.nim, line 30. Theory: the for loop should be skipped if h is zero. I will test this.

kevinmatthes commented 1 year ago

@fox0430

My theory was wrong. I still fails.

fox0430 commented 1 year ago

@kevinmatthes

Maybe I found the cause.

It may be because the suggestIndex is not updated here.

https://github.com/fox0430/moe/blob/8804d7371b2456d1ff0795ba5ebbedc0f28ac583/src/moepkg/mainloop.nim#L129-L134

fox0430 commented 1 year ago

@kevinmatthes

It seems to be fixed in https://github.com/fox0430/moe/commit/fd2caba15231f47e95824e6747d6a56713813448