Skip to content
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

Fix subfolder git detection #1715

Merged
merged 4 commits into from
Jan 27, 2021
Merged
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
4 changes: 2 additions & 2 deletions lib/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export class Commands {
}
}

export function showDebug(standardLinters: Array<Linter>, indieLinters: Array<IndieDelegate>, uiProviders: Array<UI>) {
export async function showDebug(standardLinters: Array<Linter>, indieLinters: Array<IndieDelegate>, uiProviders: Array<UI>) {
if (!manifest) {
manifest = require('../package.json')
}
Expand All @@ -104,7 +104,7 @@ export function showDebug(standardLinters: Array<Linter>, indieLinters: Array<In
const ignoreGlob = atom.config.get('linter.ignoreGlob')
const ignoreVCSIgnoredPaths = atom.config.get('core.excludeVcsIgnoredPaths')
const disabledLinters = atom.config.get('linter.disabledProviders').map(formatItem).join('\n')
const filePathIgnored = Helpers.isPathIgnored(textEditor.getPath(), ignoreGlob, ignoreVCSIgnoredPaths)
const filePathIgnored = await Helpers.isPathIgnored(textEditor.getPath(), ignoreGlob, ignoreVCSIgnoredPaths)

atom.notifications.addInfo('Linter Debug Info', {
detail: [
Expand Down
19 changes: 8 additions & 11 deletions lib/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import arrayUnique from 'lodash/uniq'
import { Range, Point } from 'atom'
import { Directory, Range, Point } from 'atom'
import type { TextEditor } from 'atom'
import type { Linter, Message, MessageSolution } from './types'

Expand All @@ -24,21 +24,18 @@ export function getEditorCursorScopes(textEditor: TextEditor): Array<string> {
}

let minimatch: typeof import('minimatch')
export function isPathIgnored(filePath: string | null | undefined, ignoredGlob: string, ignoredVCS: boolean): boolean {
export async function isPathIgnored(
filePath: string | null | undefined,
ignoredGlob: string,
ignoredVCS: boolean,
): Promise<boolean> {
if (!filePath) {
return true
}

if (ignoredVCS) {
let repository = null
const projectPaths = atom.project.getPaths()
for (let i = 0, { length } = projectPaths; i < length; ++i) {
const projectPath = projectPaths[i]
if (filePath.indexOf(projectPath) === 0) {
repository = atom.project.getRepositories()[i]
break
}
}
Comment on lines -33 to -41
Copy link
Collaborator

@aminya aminya Jan 27, 2021

Choose a reason for hiding this comment

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

Don't we need to check the project paths one by one anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, those cases are already covered by atom.project.repositoryForDirectory.

const directory = new Directory(filePath)
const repository = await atom.project.repositoryForDirectory(directory)
if (repository && repository.isPathIgnored(filePath)) {
return true
}
Expand Down
4 changes: 2 additions & 2 deletions lib/linter-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ export default class LinterRegistry {
if (
(onChange && !this.lintOnChange) || // Lint-on-change mismatch
// Ignored by VCS, Glob, or simply not saved anywhere yet
Helpers.isPathIgnored(editor.getPath(), this.ignoreGlob, this.ignoreVCS) ||
(!this.lintPreviewTabs && atom.workspace.getActivePane().getPendingItem() === editor) // Ignore Preview tabs
(!this.lintPreviewTabs && atom.workspace.getActivePane().getPendingItem() === editor) || // Ignore Preview tabs
(await Helpers.isPathIgnored(editor.getPath(), this.ignoreGlob, this.ignoreVCS))
) {
return false
}
Expand Down
2 changes: 1 addition & 1 deletion lib/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class Linter {
this.registryUIInit()
this.registryIndieInit()
this.registryLintersInit()
showDebug(
await showDebug(
// this.registryLinters becomes valid inside registryLintersInit
this.registryLinters!.getProviders(),
// this.registryIndie becomes valid inside registryIndieInit
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,13 @@
"eslint": "^7.17.0",
"eslint-config-atomic": "^1.5.1",
"eslint-plugin-react": "^7.22.0",
"fs-plus": "^3.1.1",
"jasmine-fix": "^1.3.1",
"prettier": "^2",
"rollup": "^2.35.1",
"rollup-plugin-atomic": "^1.6.4",
"shx": "^0.3.3",
"temp": "^0.9.4",
"typescript": "^4.1.3"
},
"package-deps": [
Expand Down
57 changes: 57 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions spec/fixtures/git-dir/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ignore.txt
File renamed without changes.
1 change: 1 addition & 0 deletions spec/fixtures/git-dir/git.git/HEAD
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ref: refs/heads/main
Binary file added spec/fixtures/git-dir/git.git/index
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
1 change: 1 addition & 0 deletions spec/fixtures/git-dir/git.git/refs/heads/main
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
52c5f46c008e6eaf3dc121df85a81ee0bef44a7e
1 change: 1 addition & 0 deletions spec/fixtures/git-dir/ignore.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Beep Boop
112 changes: 66 additions & 46 deletions spec/helpers-spec.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
/* @flow */

import fs from 'fs-plus'
import { it } from 'jasmine-fix'
import * as path from 'path'
import * as temp from 'temp'

import * as Helpers from '../dist/helpers'
import { getFixturesPath, getMessage } from './common'
import { getMessage } from './common'

describe('Helpers', function () {
// NOTE: Did *not* add specs for messageKey and messageKeyLegacy on purpose
Expand Down Expand Up @@ -90,55 +94,71 @@ describe('Helpers', function () {
).toBe(false)
})
})
describe('isPathIgnored', function () {
describe('isPathIgnored: ignoredGlob', function () {
function isPathIgnored(a: any, b: any, c: any) {
return Helpers.isPathIgnored(a, b || '**/*.min.{js,css}', c || false)
}

it('returns false if path does not match glob', function () {
expect(isPathIgnored('a.js')).toBe(false)
expect(isPathIgnored('a.css')).toBe(false)
expect(isPathIgnored('/a.js')).toBe(false)
expect(isPathIgnored('/a.css')).toBe(false)
})
it('returns false correctly for windows styled paths', function () {
expect(isPathIgnored('a.js')).toBe(false)
expect(isPathIgnored('a.css')).toBe(false)
expect(isPathIgnored('\\a.js')).toBe(false)
expect(isPathIgnored('\\a.css')).toBe(false)
})
it('returns true if path matches glob', function () {
expect(isPathIgnored('a.min.js')).toBe(true)
expect(isPathIgnored('a.min.css')).toBe(true)
expect(isPathIgnored('/a.min.js')).toBe(true)
expect(isPathIgnored('/a.min.css')).toBe(true)
})
it('returns true correctly for windows styled paths', function () {
expect(isPathIgnored('a.min.js')).toBe(true)
expect(isPathIgnored('a.min.css')).toBe(true)
expect(isPathIgnored('\\a.min.js')).toBe(true)
expect(isPathIgnored('\\a.min.css')).toBe(true)
})
it('returns true if the path is ignored by VCS', async function () {
try {
await atom.workspace.open(__filename)
expect(isPathIgnored(getFixturesPath('ignored.txt'), null, true)).toBe(true)
} finally {
atom.workspace.destroyActivePane()
}
})
it('returns false if the path is not ignored by VCS', async function () {
try {
await atom.workspace.open(__filename)
expect(isPathIgnored(getFixturesPath('file.txt'), null, true)).toBe(false)
} finally {
atom.workspace.destroyActivePane()
}
})
it('returns true if no path is given', function () {
expect(isPathIgnored(undefined)).toBe(true)
expect(isPathIgnored(null)).toBe(true)
expect(isPathIgnored('')).toBe(true)
it('returns false if path does not match glob', async function () {
expect(await isPathIgnored('a.js')).toBe(false)
expect(await isPathIgnored('a.css')).toBe(false)
expect(await isPathIgnored('/a.js')).toBe(false)
expect(await isPathIgnored('/a.css')).toBe(false)
})
it('returns false correctly for windows styled paths', async function () {
expect(await isPathIgnored('a.js')).toBe(false)
expect(await isPathIgnored('a.css')).toBe(false)
expect(await isPathIgnored('\\a.js')).toBe(false)
expect(await isPathIgnored('\\a.css')).toBe(false)
})
it('returns true if path matches glob', async function () {
expect(await isPathIgnored('a.min.js')).toBe(true)
expect(await isPathIgnored('a.min.css')).toBe(true)
expect(await isPathIgnored('/a.min.js')).toBe(true)
expect(await isPathIgnored('/a.min.css')).toBe(true)
})
it('returns true correctly for windows styled paths', async function () {
expect(await isPathIgnored('a.min.js')).toBe(true)
expect(await isPathIgnored('a.min.css')).toBe(true)
expect(await isPathIgnored('\\a.min.js')).toBe(true)
expect(await isPathIgnored('\\a.min.css')).toBe(true)
})
it('returns true if no path is given', async function () {
expect(await isPathIgnored(undefined)).toBe(true)
expect(await isPathIgnored(null)).toBe(true)
expect(await isPathIgnored('')).toBe(true)
})
})

describe('isPathIgnored: ignoredVCS', function () {
function isPathIgnored(a: any, b: any, c: any) {
return Helpers.isPathIgnored(a, b || '**/*.min.{js,css}', c || true)
}

let workingDir
beforeEach(() => {
workingDir = temp.mkdirSync('helpers-spec')
fs.copySync(path.join(__dirname, 'fixtures'), workingDir)
fs.moveSync(path.join(workingDir, 'git-dir', 'git.git'), path.join(workingDir, 'git-dir', '.git'))

waitsForPromise(() => atom.workspace.open(path.join(workingDir, 'git-dir', 'file.txt')))
})

it('returns true if the path is ignored by VCS', async () => {
atom.project.setPaths([path.join(workingDir, 'git-dir')])
expect(await isPathIgnored(path.join(workingDir, 'git-dir', 'ignore.txt'))).toBe(true)
})
it('returns false if the path is not ignored by VCS', async () => {
atom.project.setPaths([path.join(workingDir, 'git-dir')])
expect(await isPathIgnored(path.join(workingDir, 'git-dir', 'file.txt'))).toBe(false)
})
it('returns true if the path is ignored by VCS with the git directory in a subfolder', async () => {
atom.project.setPaths([workingDir])
expect(await isPathIgnored(path.join(workingDir, 'git-dir', 'ignore.txt'))).toBe(true)
})
it('returns false if the path is not ignored by VCS with the git directory in a subfolder', async () => {
atom.project.setPaths([workingDir])
expect(await isPathIgnored(path.join(workingDir, 'git-dir', 'file.txt'))).toBe(false)
})
})

Expand Down