fix(tui): refresh workspace before reloading#12831
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
bot
added
rust
There was a problem hiding this comment.
Pull request overview
Fixes TUI commit rewording failing on second attempt by refreshing the workspace before reloading status, and simplifies error handling by replacing the TuiResultExt trait with ? operator propagation.
Changes:
- Added
ws.refresh_from_head()call in theReloadmessage handler to refresh internal caches - Replaced
TuiResultExterror handling with standard?operator by extractingtry_handle_message - Removed
error.rsmodule, inliningAppErrorstruct intomod.rs
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
crates/but/src/command/legacy/status/tui/mod.rs |
Extracted try_handle_message, added workspace refresh on reload, moved AppError inline |
crates/but/src/command/legacy/status/tui/error.rs |
Deleted; AppError moved, TuiResultExt removed |
You can also share your feedback on Copilot code review. Take the survey.
| { | ||
| let meta = ctx.meta()?; | ||
| let (_guard, repo, mut ws, _) = ctx.workspace_mut_and_db()?; | ||
| ws.refresh_from_head(&repo, &meta)?; | ||
| } |
There was a problem hiding this comment.
This is the actual interesting bit, the rest is just plumbing for error handling.
There was a problem hiding this comment.
(I only looked at this hunk specifically)
Yes, that's the easy way let the cached workspace representation update itself. When but-api functions are used, the expectation is that they do this for you, or in other ways leave you with a workspace that is in sync with repo.
When calling plumbing, this won't be the case.
The reason you are actually seeing this, which I think is good as we can 'practice', is that probably the Context is long-lived, which has the benefit of hot caches and overall, can lead to better performance.
However, there is definitely one more caveat: the gix::Repository won't auto-update its snapshots for Git configuration yet, so that will go stale if any other mechanism of config updates is used than to go through repo.config_snapshot_mut(). I am working on rectifying this by switching everything over to gix, see #12747 .
Edit: Actually, this also isn't always true as there is code that can open configuration files directly. So I presume it's something to be aware of, and maybe might lead to a concept of recreating the ctx.repo every now and then, or maybe I could at least add a way to refresh the snapshot 'brutally'. Yeah, let me add that to gix.
davidpdrsn
force-pushed
the
dp/slnromkoovwo
branch
2 times, most recently
from
8b4d53d to
3f6f68a
Compare
There was a problem hiding this comment.
Pull request overview
Fixes a TUI bug where rewording the same commit twice would fail by refreshing the workspace before reloading status. Also simplifies error handling by replacing the TuiResultExt trait with ? operator and a wrapper in handle_message.
Changes:
- Added
ws.refresh_from_head()call in theMessage::Reloadhandler to refresh workspace caches - Replaced
TuiResultExterror-handling trait with?operator by splittinghandle_messageinto a try/catch wrapper pattern - Moved
AppErrorstruct inline and deleted theerror.rsmodule
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
crates/but/src/command/legacy/status/tui/mod.rs |
Added workspace refresh on reload, refactored error handling to use ?, inlined AppError struct |
crates/but/src/command/legacy/status/tui/error.rs |
Deleted - TuiResultExt trait removed in favor of ? operator |
You can also share your feedback on Copilot code review. Take the survey.
davidpdrsn
force-pushed
the
dp/slnromkoovwo
branch
from
3f6f68a to
f1a3472
Compare
I've been working on rewording commits from the TUI, but if you reworded the same commit twice, the second time it would fail, saying it couldn't find the commit. It seems as if there's some internal caches that needs to be refreshed when we'd reload the status.
This does fix the issues that I got with rewarding the same commit multiple times, but I'm not really sure if this is the proper fix.