Repository navigation
[US-1960] Reload from disk and show when the open file has changed #35
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
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
a7c4bbf
feat: reload from disk and on-disk change detection
3ace 1c4be12
test: temporary Windows diagnostic for the busy-file lock
3ace 59491c3
test: check backup semantics in the Windows lock diagnostic
3ace 12729ec
fix: report a byte-range-locked file as busy on read; lock test files…
3ace 98e2ff8
fix: signatures read the file only for signed fields, stop at the fir…
3ace 7055508
fix: reload keeps focus on the tab you switched to, re-checks the dis…
3ace 4c77613
fix: Open parses again when the file changes mid-parse; a recorded di…
3ace 356c073
fix: Diff-header reload errors and warnings follow the reload path; d…
3ace d4132b7
fix: watcher settles on state, resyncs after disk checks and keeps me…
3ace 7ac6281
fix: swallowed Cmd/Ctrl+R presses drop their menu echo; per-signature…
3ace File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| // A file rewritten after the CLI parsed it is reported instead of mixing the | ||
| // old parse with the new bytes: dump bytes (raw and --json) exits 2 with a | ||
| // stderr JSON error; dump signatures keeps the parsed details and prints the | ||
| // hole check as not checked. afterCLIOpen runs right after openForCLI | ||
| // succeeds and rewrites the file. | ||
| // | ||
| // Run: cd code && go test -run TestCLIFileChanged ./cmd/cli/ | ||
|
|
||
| package main | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
| "testing" | ||
| "time" | ||
| ) | ||
|
|
||
| // signedCopy copies testdata/signed.pdf into a temp dir. | ||
| func signedCopy(t *testing.T) string { | ||
| t.Helper() | ||
| data, err := os.ReadFile(filepath.Join("..", "..", "testdata", "signed.pdf")) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| path := filepath.Join(t.TempDir(), "signed.pdf") | ||
| if err := os.WriteFile(path, data, 0o644); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| return path | ||
| } | ||
|
|
||
| // rewriteAfterOpen installs an afterCLIOpen hook that overwrites the opened | ||
| // file with minimal.pdf and moves its mtime ahead. | ||
| func rewriteAfterOpen(t *testing.T) *bool { | ||
| t.Helper() | ||
| called := new(bool) | ||
| afterCLIOpen = func(path string) { | ||
| *called = true | ||
| data, err := os.ReadFile(filepath.Join("..", "..", "testdata", "minimal.pdf")) | ||
| if err != nil { | ||
| t.Errorf("read minimal.pdf: %v", err) | ||
| return | ||
| } | ||
| if err := os.WriteFile(path, data, 0o644); err != nil { | ||
| t.Errorf("rewrite: %v", err) | ||
| } | ||
| later := time.Now().Add(time.Hour) | ||
| _ = os.Chtimes(path, later, later) | ||
| } | ||
| t.Cleanup(func() { afterCLIOpen = nil }) | ||
| return called | ||
| } | ||
|
|
||
| func TestCLIFileChangedBytesJSON(t *testing.T) { | ||
| path := signedCopy(t) | ||
| called := rewriteAfterOpen(t) | ||
| var code int | ||
| var stderr string | ||
| captureStdout(t, func() { | ||
| stderr = captureStderr(t, func() { | ||
| code = execBytesDump(path, docViewFlags{json: true}) | ||
| }) | ||
| }) | ||
| if !*called { | ||
| t.Fatalf("afterCLIOpen was not called by dump bytes --json") | ||
| } | ||
| if code != 2 { | ||
| t.Errorf("exit %d, want 2", code) | ||
| } | ||
| if !strings.Contains(stderr, "file changed on disk") { | ||
| t.Errorf("stderr %q must contain %q", stderr, "file changed on disk") | ||
| } | ||
| } | ||
|
|
||
| func TestCLIFileChangedBytesRaw(t *testing.T) { | ||
| path := signedCopy(t) | ||
| called := rewriteAfterOpen(t) | ||
| var code int | ||
| var stdout, stderr string | ||
| stdout = captureStdout(t, func() { | ||
| stderr = captureStderr(t, func() { | ||
| code = execBytesDump(path, docViewFlags{}) | ||
| }) | ||
| }) | ||
| if !*called { | ||
| t.Fatalf("afterCLIOpen was not called by dump bytes") | ||
| } | ||
| if code != 2 { | ||
| t.Errorf("exit %d, want 2", code) | ||
| } | ||
| if !strings.Contains(stderr, "file changed on disk") { | ||
| t.Errorf("stderr %q must contain %q", stderr, "file changed on disk") | ||
| } | ||
| if stdout != "" { | ||
| t.Errorf("no bytes of the rewritten file may reach stdout, got %d bytes", len(stdout)) | ||
| } | ||
| } | ||
|
|
||
| func TestCLIFileChangedSignatures(t *testing.T) { | ||
| path := signedCopy(t) | ||
| called := rewriteAfterOpen(t) | ||
| var code int | ||
| stdout := captureStdout(t, func() { | ||
| captureStderr(t, func() { | ||
| code = execSignaturesDump(path, docViewFlags{}) | ||
| }) | ||
| }) | ||
| if !*called { | ||
| t.Fatalf("afterCLIOpen was not called by dump signatures") | ||
| } | ||
| if code != 0 { | ||
| t.Errorf("exit %d, want 0", code) | ||
| } | ||
| if !strings.Contains(stdout, "not checked: file changed on disk") { | ||
| t.Errorf("stdout must note the hole as not checked, got:\n%s", stdout) | ||
| } | ||
| if !strings.Contains(stdout, "ByteRange") { | ||
| t.Errorf("stdout must keep the parsed signature details, got:\n%s", stdout) | ||
| } | ||
| } | ||
|
|
||
| func TestCLIFileChangedHookUnsetLeavesRunsUnaffected(t *testing.T) { | ||
| path := signedCopy(t) | ||
| if afterCLIOpen != nil { | ||
| t.Fatalf("afterCLIOpen must be nil outside tests") | ||
| } | ||
| var code int | ||
| var stderr string | ||
| captureStdout(t, func() { | ||
| stderr = captureStderr(t, func() { | ||
| code = execSignaturesDump(path, docViewFlags{json: true}) | ||
| }) | ||
| }) | ||
| if code != 0 || stderr != "" { | ||
| t.Errorf("unchanged file: exit %d stderr %q, want 0 and nothing", code, stderr) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Only the
--jsonbranch goes through a fingerprint-checked read. The default raw path further down (os.Open(filePath)+io.Copy, line 78) streams the file with no check. If the file is rewritten afteropenForCLI, which is exactly what this hook simulates,execBytesDump(path, docViewFlags{})exits 0 and prints the new bytes, while--jsonexits 2 with "file changed on disk". The description saysdump bytespassesErrFileChangedthrough, but that holds only for--json.TestCLIFileChanged*only coversjson: true.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.
Fixed in d4132b7. The raw path now goes through a new
Inspector.CopyDocumentFile, which checks the handle against the Open fingerprint before the first byte and after the last, and maps a lock violation toErrFileBusy. Rawdump byteson a rewritten file exits 2 with "file changed on disk" and writes nothing (TestCLIFileChangedBytesRaw).