This PR makes the columnar and description menus update their lists of suggestions after processing events (in update_working_details), rather than before. Currently, these menus can be rendered with outdated working details.
The completions example in Reedline itself demonstrates this. Here's how it looks currently (I modified the default completer to add a dummy description just for this recording):
Here's how the columnar menu looks after this PR:
Description menu (before):
Description menu (after):
I assume there was a reason for originally updating working details before processing events, given that both the columnar and description menu have comments explicitly saying that that's what they're doing. However, reading the code, I didn't see any reason to believe that processing the events first would mess with the working details, I didn't run into any problems myself, and the new IDE menu made by maxomatic458 (which processes events before updating working details) seems to work just fine. If anyone knows of any possible problems with processing events first, do let me know, but otherwise, I think this PR is safe to merge.
fdncred
commented
4 months ago
This PR makes the columnar and description menus update their lists of suggestions after processing events (in
update_working_details), rather than before. Currently, these menus can be rendered with outdated working details.The completions example in Reedline itself demonstrates this. Here's how it looks currently (I modified the default completer to add a dummy description just for this recording):
Here's how the columnar menu looks after this PR:
Description menu (before):
Description menu (after):
I assume there was a reason for originally updating working details before processing events, given that both the columnar and description menu have comments explicitly saying that that's what they're doing. However, reading the code, I didn't see any reason to believe that processing the events first would mess with the working details, I didn't run into any problems myself, and the new IDE menu made by maxomatic458 (which processes events before updating working details) seems to work just fine. If anyone knows of any possible problems with processing events first, do let me know, but otherwise, I think this PR is safe to merge.