-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reordered prompt done callback to avoid accessing out of bound history #3318
Reordered prompt done callback to avoid accessing out of bound history #3318
Conversation
More precisely, what happens is:
i.History[ptype] = append(i.History[ptype], "")
h := i.History[i.PromptType]
h[len(h)-1] = resp since it doesn't realize that
Anyway, your fix looks correct. |
@@ -160,6 +159,7 @@ func (i *InfoBuf) DonePrompt(canceled bool) { | |||
break | |||
} | |||
} | |||
i.PromptCallback(resp, false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: add empty line above this line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove spaces from this empty line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
434eec8
to
26f192c
Compare
Merged, thanks. |
According to https://pkg.go.dev/github.com/zyedidia/micro/v2@v2.0.13/internal/info#InfoBuf.Prompt
A lua script can provide a done callback when the prompt has finished (enter is pressed).
This works fine until the done callback itself wants to have another prompt, for example multiple questions to configure an action.
The behavior in main right now will crash with
So if I have something like this
It won't work and crash inside the 2nd prompt.
This is due to the 2nd call in the prompt trying to access history while the first prompt has not finished modifying the history.
See
micro/internal/info/infobuffer.go
Lines 97 to 109 in e9bd1b3
and
micro/internal/info/infobuffer.go
Lines 150 to 154 in e9bd1b3
where line 152 is calling the done callback and we are still modifying the history for the current prompt on line 153+
The proposed solution just needs to reorder the done callback to the end of the block where we have finished modifying the history so that the next prompt call can safely modify it.