Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Don’t add unwanted/overly broad .gitignore entries #2317

Closed
klmr opened this issue Feb 6, 2021 · 3 comments
Closed

Don’t add unwanted/overly broad .gitignore entries #2317

klmr opened this issue Feb 6, 2021 · 3 comments

Comments

@klmr
Copy link
Contributor

klmr commented Feb 6, 2021

build_vignettes helpfully adds the entries doc and Meta to .gitignore, if they don’t already exist.

However, more specific, existing entries are ignored. In particular, if the .gitignore file contains /doc/ and /Mate/, the (redundant) entries are still added.

Unfortunately, my project is actually using a nested doc directory, so that project doesn’t want to ignore this name.

I’m happy to contribute a patch but I’m not sure how to do this well: ideally the build_vignettes should probably be maximally specific, and attempt to add /doc/ and /Meta/ instead of doc and Meta to avoid falsely matching unrelated directories. But such a change wouldn’t be backwards compatible (i.e. existing ‘devtools’ projects would now get redundant entries added to .gitignore) so it doesn’t seem viable. Instead, maybe an option to skip automatic additions to .gitignore, for users who rely on such directory names, might be conceivable.

@jimhester
Copy link
Member

I think you should just not use build_vignettes() then.

@klmr
Copy link
Contributor Author

klmr commented Feb 8, 2021

To my horror, a friend let me know that my initial comment sounds quite aggressive. Please accept my apology, that truly wasn’t my intention and I hadn’t noticed the pushy phrasing.

To clarify, the behaviour of build_vignettes (including writing .gitignore entries) is helpful, and I’m not blaming it for causing clashes in my own projects. I’m keen on helping to improve it, and I believe that the issue I observe might also manifest for other users (Meta is a very specific name, but having a doc subdirectory is probably not that uncommon).

Before submitting this issue my intent was to directly submit a PR with a candidate fix. But unfortunately this isn’t trivial, since merely substituting the strings (doc/doc/, Meta/Meta/) would cause trouble for existing projects using ‘devtools’ which already have doc and Meta entries in their .gitignore. So instead I submitted this issue, to figure out a viable way of improving the function. Please read the “demands” in my initial message as suggestions to open a discussion — that’s how they were intended.

@jimhester
Copy link
Member

I think updating it to use more strict /doc/ and /Meta/ seems line a good improvement, would be happy to accept a PR with that change.

The logic for writing the union lives in the usethis package, however I am not sure attempting to remove redundant non-exact matches is a good idea, I think there may be edge cases where people would not want this behavior.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants