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

Use --exclude-from=.gitignore in place of --exclude-standard #17399

Merged
merged 1 commit into from
Jun 16, 2022

Conversation

precondition
Copy link
Contributor

@precondition precondition commented Jun 16, 2022

Description

The lint check spews an error anytime it encounters a file that's listed in the output of git_get_ignored_files. However, the use of --exclude-standard in the git shell command ran by the aforementioned python function also ignores files in .git/info/exclude and the user's global .gitignore.

$ man git ls-files
[snip]
       -X <file>, --exclude-from=<file>
           Read exclude patterns from <file>; 1 per line.

       --exclude-per-directory=<file>
           Read additional exclude patterns that apply only to the directory and
           its subdirectories in <file>.

       --exclude-standard
           Add the standard Git exclusions: .git/info/exclude, .gitignore in
           each directory, and the user’s global exclusion file.
[snip]

This has the undesirable consequence of always failing the lint check if the user added some of his keyboard or keymap folders in a personal git ignore file like .git/info/exclude or his global .gitignore.

As explained in the docstring of the function, the intent of the function is only to "return a list of files that would be captured by the current .gitignore". To actually achieve that and avoid false negatives, using the option exclude-from=.gitignore is more appropriate.

Note: exclude-from=.gitignore works even when the check_dir is not the root directory

~/qmk_firmware on stricter_git_ignored_files *3 ⨤3                                
❯ cat .git/info/exclude
# git ls-files --others --exclude-from=.git/info/exclude
# Lines that start with '#' are comments.
# For a project mostly in C, the following would be a good set of
# exclude patterns (uncomment them if you want to use them):
# *.[oa]
# *~
keyboards/handwired/dactyl_manuform/5x6/keymaps/precondition
keyboards/polilla/rev1/keymaps/precondition
keyboards/splitography

~/qmk_firmware on stricter_git_ignored_files *3 ⨤3
❯ cd keyboards/handwired/dactyl_manuform/5x6

~/qmk_firmware/k/handw/dactyl_manuform/5x6 on stricter_git_ignored_files *3 ⨤3 
❯ touch via.json

~/qmk_firmware/k/handw/dactyl_manuform/5x6 on stricter_git_ignored_files *3 ⨤3    
❯ git ls-files -c -o -i --exclude-standard  
keymaps/precondition
via.json

~/qmk_firmware/k/handw/dactyl_manuform/5x6 on stricter_git_ignored_files *3 ⨤3    
❯ git ls-files -c -o -i --exclude-from=.gitignore
via.json

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@github-actions github-actions bot added cli qmk cli command python labels Jun 16, 2022
@tzarc tzarc requested review from zvecr and a team June 16, 2022 06:53
@zvecr zvecr requested a review from a team June 16, 2022 17:12
@drashna drashna merged commit 0b1bed1 into qmk:develop Jun 16, 2022
@precondition precondition deleted the stricter_git_ignored_files branch June 17, 2022 06:28
0xcharly pushed a commit to Bastardkb/bastardkb-qmk that referenced this pull request Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli qmk cli command python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants