Skip to content

Commit

Permalink
Fix diff view provider using wrong ctx for cat
Browse files Browse the repository at this point in the history
Summary:
We were using the ctx value for `cat` commands that was used when initializing the repo. However, if you change your cwd, you may get the "wrong" cwd when running cat, which fails. The right side of the comparison just uses the file URI, which is fine.

instead, we can just use the initial connection ctx for the repo, which we know will be valid for the repo in vscode.

Differential Revision: D55146168

fbshipit-source-id: d9678708540979c4375f5875825f088cac132005
  • Loading branch information
evangrayk authored and facebook-github-bot committed Mar 20, 2024
1 parent 124e16c commit a063a87
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
5 changes: 4 additions & 1 deletion addons/vscode/extension/DiffContentProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,12 @@ export class SaplingDiffContentProvider implements vscode.TextDocumentContentPro

const revset = revsetForComparison(data.comparison);

// Ensure we use a ctx appropriate for this repo. `this.ctx` may be an unrelated cwd.
const ctx = repo?.initialConnectionContext;

// fall back to fetching from the repo
const fetchedFileContent = await repo
.cat(this.ctx, fsPath, revset)
.cat(ctx, fsPath, revset)
// An error during `cat` usually means the right side of the comparison was added since the left,
// so `cat` claims `no such file` at that revset.
// TODO: it would be more accurate to check that the error is due to this, and return null if not.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ function mockRepoAdded(): NonNullable<typeof activeRepo> {
mockChangeHeadCommit(commit: CommitInfo) {
savedOnChangeHeadCommit(commit);
},
initialConnectionContext: {
cwd: '/path/to/repo',
},
} as unknown as typeof activeRepo;
activeReposCallback?.([nullthrows(activeRepo)]);
return nullthrows(activeRepo);
Expand Down Expand Up @@ -111,7 +114,7 @@ describe('DiffContentProvider', () => {

expect(content).toEqual(FILE1_CONTENT_HEAD);
expect(repo.cat).toHaveBeenCalledTimes(1);
expect(repo.cat).toHaveBeenCalledWith(ctx, '/path/to/repo/file1.txt', '.');
expect(repo.cat).toHaveBeenCalledWith({cwd: '/path/to/repo'}, '/path/to/repo/file1.txt', '.');
provider.dispose();
});

Expand Down

0 comments on commit a063a87

Please sign in to comment.