desertbit/grumble

Crash when using with search + complete

Closed this issue · 9 comments

Hi,

We are using grumble on our CLI, when mixing the back-search (CTRL+R) with the complete from Grumble we get a crash:

sliver > panic: runtime error: index out of range [-1] win32_service"'
bck-i-search: shar
goroutine 7 [running]:
github.com/desertbit/grumble.(*completer).Do(0xc0000b0728, 0xc001499100, 0x3d, 0x3d, 0x0, 0x3d, 0xc0080a6900, 0xc00681bcb0, 0x100c5f8)
	/Users/xxx/go/pkg/mod/github.com/desertbit/grumble@v1.0.5/completer.go:52 +0x16ea
github.com/desertbit/readline.(*opCompleter).OnComplete(0xc0072ce1c0, 0xc00685d9e0)
	/Users/xxx/go/pkg/mod/github.com/desertbit/readline@v0.0.0-20171208011716-f6d7a1f6fbf3/complete.go:87 +0x142
github.com/desertbit/readline.(*Operation).ioloop(0xc0072ce0e0)
	/Users/xxx/go/pkg/mod/github.com/desertbit/readline@v0.0.0-20171208011716-f6d7a1f6fbf3/operation.go:178 +0x10a4
created by github.com/desertbit/readline.NewOperation
	/Users/xxx/go/pkg/mod/github.com/desertbit/readline@v0.0.0-20171208011716-f6d7a1f6fbf3/operation.go:88 +0x2b1

Steps to reproduce:

  • CTRL+R and start writing "sharp"
  • Press tab to autocomplete

I fixed that changing the line 52 from completer.go to:

if len(words) > 0 && pos > 1 && len(line) > pos && line[pos-1] != ' ' {

This fix should avoid negative indexes on any case. What do you think? Should I do a PR? I'm not sure if the fix is good enough, maybe could be better to parse "TAB" or something like that.

Thanks

Hi,

I could reproduce your bug by doing the following:

  • go run sample/simple/main.go
  • run the daemon command once
  • then press Ctrl+r for back-search and enter for example daem
  • hit Tab
  • crash

To me, it seems like it is critical that the command that will be completed by the back-search is also in the command history of grumble. I could not reproduce the bug with a fresh start and directly doing the back-search. I had to run the command in question at least once. Can you confirm that @h4ng3r?

@r0l1 and I will take a look at your suggested changes, thank you for submitting and reporting the issue, very much appreciated.

Hi @skaldesh,

Yep that's true you need a match in the back search, you need to hit TAB on a match otherwise it won't crash. Nice catch.

Thanks,

@h4ng3r I played around with your proposed fix, but I think the len(line) > pos prevents the auto completion for me to work generally. Typing dae and hitting Tab then does no longer auto complete, since the line is dae, pos is 2 (the end) and the expression evaluates to false then.

In addition, when using the backsearch, which then autocompletes daemon, position is still 0, which causes grumble to append a dash - in front of daemon. This happens, since the auto completer is presented with the line daemon and therefore wants to start suggesting flags... I will propose some solution for this soon

@skaldesh yep, that's true I realized yesterday that my solution was disabling auto completion. Still not sure how to fix it properly, sorry.

No problem, will play around a little bit, thank you for the quick answers :D

I was being stupid. This bug has nothing to do with backsearch. The problem is that grumble can not handle the case that somebody types a command, say daemon, then moves the cursor back to the start of the word and hits Tab. This crashes as well. Good thing you discovered this @h4ng3r

@h4ng3r I created a PR. Can you, @h4ng3r , check it out and see, if it fixes your problem and hopefully does not cause any further bugs? :)

I tested the PR, the issue is fixed and the autocomplete is working perfectly. Looks good to me.

Thank you for the help again. The issue has been fixed in master