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

Fix subfolder git detection #1715

merged 4 commits into from
Jan 27, 2021

Conversation

utkarshgupta137
Copy link
Contributor

@utkarshgupta137 utkarshgupta137 commented Nov 5, 2020

This change affects the "Ignore VCS" option.

Description

If you add a project folder which is not a git repo but it contains folders which are git repos, then the repository detection doesn't work.

Steps to Reproduce

  1. Create the following tree:
    |-- mainFolderWithoutGit
    .....|-- someFolderWithGit
    ..........|-- .git
  2. Add mainFolderWithoutGit as a project.
  3. Open a git-ignored file from someFolderWithGit

Current Behaviour:

The "Ignore VCS" option doesn't work, and the file is still linted.

Expected behavior:

The file should not be linted if the "Ignore VCS" option is enabled.

Solution:

The repositoryForDirectory function in atom.project is able to detect the repository for any Directory, even if it is in a subfolder.
It is an async function, so some changes have to be made in up the call chain.

Also updated the tests to test it.

@utkarshgupta137
Copy link
Contributor Author

TODO: Add tests

@aminya
Copy link
Collaborator

aminya commented Jan 4, 2021

@utkarshgupta137 I changed the codebase to TypeScript. Do you want to move the changes to the new files?

@utkarshgupta137
Copy link
Contributor Author

@aminya done.

@aminya
Copy link
Collaborator

aminya commented Jan 22, 2021

Could you add some description about the bug this is fixing?

@utkarshgupta137
Copy link
Contributor Author

@aminya done.

Copy link
Collaborator

@aminya aminya left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation. I will use this branch for some time to see if everything works as expected.

Comment on lines -33 to -41
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
}
}
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants