Skip to content

Keep tracked files instead of refreshing list of untracked files #61

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 3 additions & 18 deletions src/git.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
getDiffFileList,
getDiffForFile,
getRangesForDiff,
getUntrackedFileList,
getTrackedFileList,
hasCleanIndex,
} from "./git";
import {
Expand Down Expand Up @@ -142,33 +142,18 @@ describe("getDiffFileList", () => {
});
});

describe("getUntrackedFileList", () => {
describe("getTrackedFileList", () => {
it("should get the list of untracked files", () => {
jest.mock("child_process").resetAllMocks();
mockedChildProcess.execFileSync.mockReturnValueOnce(
Buffer.from(diffFileList)
);
expect(mockedChildProcess.execFileSync).toHaveBeenCalledTimes(0);
const fileListA = getUntrackedFileList(false);
expect(mockedChildProcess.execFileSync).toHaveBeenCalledTimes(1);

mockedChildProcess.execFileSync.mockReturnValueOnce(
Buffer.from(diffFileList)
);
const staged = false;
const fileListB = getUntrackedFileList(staged);
// `getUntrackedFileList` uses a cache, so the number of calls to
// `execFileSync` will not have increased.
const fileListA = getTrackedFileList();
expect(mockedChildProcess.execFileSync).toHaveBeenCalledTimes(1);

expect(fileListA).toEqual(
["file1", "file2", "file3"].map((p) => path.resolve(p))
);
expect(fileListA).toEqual(fileListB);
});

it("should not get a list when looking when using staged", () => {
const staged = true;
expect(getUntrackedFileList(staged)).toEqual([]);
});
});
36 changes: 12 additions & 24 deletions src/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,17 @@ const getDiffFileList = (staged: boolean): string[] => {
.map((filePath) => resolve(filePath));
};

const getTrackedFileList = (): string[] => {
const args = [ "ls-files" ];

return child_process
.execFileSync(COMMAND, args, OPTIONS)
.toString()
.trim()
.split("\n")
.map((filePath) => resolve(filePath))
};

const hasCleanIndex = (filePath: string): boolean => {
const args = [
"diff",
Expand All @@ -77,29 +88,6 @@ const fetchFromOrigin = (branch: string) => {
child_process.execFileSync(COMMAND, args, OPTIONS);
};

let untrackedFileListCache: string[] | undefined;
const getUntrackedFileList = (
staged: boolean,
shouldRefresh = false
): string[] => {
if (staged) {
return [];
}

if (untrackedFileListCache === undefined || shouldRefresh) {
const args = ["ls-files", "--exclude-standard", "--others"];

untrackedFileListCache = child_process
.execFileSync(COMMAND, args, OPTIONS)
.toString()
.trim()
.split("\n")
.map((filePath) => resolve(filePath));
}

return untrackedFileListCache;
};

const isHunkHeader = (input: string) => {
const hunkHeaderRE = /^@@ [^@]* @@/u;

Expand Down Expand Up @@ -160,6 +148,6 @@ export {
getDiffFileList,
getDiffForFile,
getRangesForDiff,
getUntrackedFileList,
getTrackedFileList,
hasCleanIndex,
};
4 changes: 2 additions & 2 deletions src/processors.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
jest.mock("./git", () => ({
...jest.requireActual<typeof git>("./git"),
getUntrackedFileList: jest.fn(),
getTrackedFileList: jest.fn(),
getDiffFileList: jest.fn(),
getDiffForFile: jest.fn(),
hasCleanIndex: jest.fn(),
Expand All @@ -19,7 +19,7 @@ const untrackedFilename = "an-untracked-file.js";

const gitMocked: jest.MockedObjectDeep<typeof git> = jest.mocked(git);
gitMocked.getDiffFileList.mockReturnValue([filename]);
gitMocked.getUntrackedFileList.mockReturnValue([untrackedFilename]);
gitMocked.getTrackedFileList.mockReturnValue([filename, "file-with-dirty-index.js"]);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the tests also works with a file called file-with-dirty-index.js so we have to add it to the "tracked" list too.


describe("processors", () => {
it("preprocess (diff and staged)", async () => {
Expand Down
22 changes: 8 additions & 14 deletions src/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
getDiffFileList,
getDiffForFile,
getRangesForDiff,
getUntrackedFileList,
getTrackedFileList,
hasCleanIndex,
} from "./git";
import type { Range } from "./Range";
Expand All @@ -28,18 +28,12 @@ if (process.env.CI !== undefined) {
* This is increasingly useful the more files there are in the repository.
*/
const getPreProcessor =
(diffFileList: string[], staged: boolean) =>
(trackedFileSet: Set<string>, diffFileList: string[]) =>
(text: string, filename: string) => {
let untrackedFileList = getUntrackedFileList(staged);
const shouldRefresh =
!diffFileList.includes(filename) && !untrackedFileList.includes(filename);
Comment on lines -34 to -35
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition not only matches new files but actually all unchanged files, so instead of speeding things up this actually slows the entire lint process.

if (shouldRefresh) {
untrackedFileList = getUntrackedFileList(staged, true);
}
const shouldBeProcessed =
process.env.VSCODE_PID !== undefined ||
diffFileList.includes(filename) ||
untrackedFileList.includes(filename);
!trackedFileSet.has(filename);

return shouldBeProcessed ? [text] : [];
};
Expand Down Expand Up @@ -72,7 +66,7 @@ const getUnstagedChangesError = (filename: string): [Linter.LintMessage] => {
};

const getPostProcessor =
(staged: boolean) =>
(trackedFileSet: Set<string>, staged: boolean) =>
(
messages: Linter.LintMessage[][],
filename: string
Expand All @@ -81,8 +75,7 @@ const getPostProcessor =
// No need to filter, just return
return [];
}
const untrackedFileList = getUntrackedFileList(staged);
if (untrackedFileList.includes(filename)) {
if (!trackedFileSet.has(filename)) {
// We don't need to filter the messages of untracked files because they
// would all be kept anyway, so we return them as-is.
return messages.flat();
Expand Down Expand Up @@ -117,11 +110,12 @@ const getProcessors = (
processorType: ProcessorType
): Required<Linter.Processor> => {
const staged = processorType === "staged";
const trackedFileSet = new Set(getTrackedFileList());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wrapping this in a Set for faster lookups.

const diffFileList = getDiffFileList(staged);

return {
preprocess: getPreProcessor(diffFileList, staged),
postprocess: getPostProcessor(staged),
preprocess: getPreProcessor(trackedFileSet, diffFileList),
postprocess: getPostProcessor(trackedFileSet, staged),
supportsAutofix: true,
};
};
Expand Down