Index fixes #1190
Index fixes #1190
Conversation
src/actions/actions.ts
Outdated
| if (prevSearchList[vimState.globalState.searchStateIndex] !== undefined) { | ||
| searchState.searchString = prevSearchList[vimState.globalState.searchStateIndex].searchString; | ||
| vimState.globalState.searchStateIndex -= 1; | ||
| } | ||
| } else if (key === "<down>") { | ||
| const prevSearchList = vimState.globalState.searchStatePrevious!; | ||
|
|
||
| // Preincrement if on boundary to prevent seeing the same search index twice | ||
| if (vimState.globalState.searchStateIndex === 0) { |
There was a problem hiding this comment.
How come we increment twice when we start at 0? I don't quite follow.
There was a problem hiding this comment.
Otherwise index is 0 and you can press up and down alternating and never see a different result
|
@johnfn this one is actually pretty solid even if it looks weird What it fixes:
|
|
@johnfn this is weird, but I would rather it be merged than closed... |
|
Sorry for taking so long on this one. I had it incorrectly categorized as a WIP in my mind, so I totally forgot to come back to it. The reason I hesitated here is this fix doesn't really make sense. The index value should be just an index into the array that you can walk up or down by pressing up or down respectively. I don't see anything about the structure of that task that requires a double increment at the borders, which makes me cautious about this PR. I would like the code to reflect the structure of the problem we try to solve. That being said, I don't really understand the search history code all too well, because I didn't implement it. So if you tell me this is the best way, I'll trust your judgement. |
|
There 100% has to be a better way with a handful of lines to do the same thing, this is just covering that up let me take a peek, I will @johnfn you when its good to go |
|
@johnfn I think that this reads better. Works even better since if you make it to the end of history you get "" so you can enter your own search The key was when entering "/" or "?" or reaching the end, allowing the index to go past the end of the limit. |
|
Alright, looks good to me now! |
A few minor fixes, fixes #1188