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

Add temporary vim files to .gitignore #4097

Closed
wants to merge 1 commit into from

Conversation

kpcyrd
Copy link

@kpcyrd kpcyrd commented May 25, 2017

The first thing I do when creating a new project is editing the .gitignore to ignore temporary vim files. This patch would make git ignore *.swp and *.swo directly after cargo new, similar to *.rs.bk.

The first thing I do when creating a new project is editing the
.gitignore to ignore temporary vim files. This patch would make git
ignore `*.swp` and `*.swo` directly after cargo new, similar to
`*.rs.bk`.
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@Mark-Simulacrum
Copy link
Member

It's probably better for you to include these in a global ignore file, for which ~/.gitignore I believe should work, instead of "polluting" Rust projects with them.

@@ -384,7 +384,7 @@ fn mk(config: &Config, opts: &MkOptions) -> CargoResult<()> {
let path = opts.path;
let name = opts.name;
let cfg = global_config(config)?;
let ignore = ["target/\n", "**/*.rs.bk\n",
let ignore = ["target/\n", "**/*.rs.bk\n", "*.sw[op]\n",
Copy link
Member

@est31 est31 May 25, 2017

Choose a reason for hiding this comment

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

Can also *-swp files be added? that would look like *[.-]sw[op] ... I think?. This would be useful for me, as Kate generates .filename.kate-swp files

@kpcyrd
Copy link
Author

kpcyrd commented May 25, 2017

After reading through some of those, it appears that **/*.rs.bk isn't editor specific as I assumed, but generated by rustfmt.

Unless somebody thinks this might be useful, I'd close this PR.

@withoutboats
Copy link
Contributor

I agree with @alexcrichton, these kind of ignores are per-user, not per-project. We'd also be opening the door to adding every kind of temp file a user could create, which would mean everyones' gitignores would be full of files they mostly don't create.

@matklad
Copy link
Member

matklad commented May 25, 2017

Yeah, ideally this should be handled by the global gitignore: https://help.github.com/articles/ignoring-files/#create-a-global-gitignore. @Mark-Simulacrum, plain ~/.gitignore won't work, one needs to also add it to ~/.gitconfig ;)

That said, I also believe this is the case where practicality beats purity, and I would be glad to add editor-specific rules to the .gitignore of the Cargo itself. I don't think it can cause any problems, and sure it can help some contributors.

But adding it to the .gitignore of any project created by Cargo goes a bit to far. It's definitely true that not all users of Cargo use vim, and for them having shorter .gitignore is definitely a benefit.

@alexcrichton
Copy link
Member

The practical side of this sort of echos with me at this point. We receive so many bug reports related to gitignore it seems that we need to do something. We could maybe add a comment to the file (pointing to the github global gitignore article) or something like that? I'm just thinking that the constant stream of bug reports about this warrants something at least.

@Mark-Simulacrum
Copy link
Member

I think a couple possibilities are available to us (perhaps in some combination):

  • Comment to documentation about global .gitignore
  • Add a cargo ignore or cargo config command that allows specifying your own template (though I don't agree with this since you shouldn't really be adding Vim's swap files and similar to this file).
  • Put up a page on Cargo's docs documenting how to customize the .gitignore and explicitly referencing the global .gitignore.

Of these, I think the first and last are my preferences.

@matklad
Copy link
Member

matklad commented May 25, 2017

We receive so many bug reports related to gitignore it seems that we need to do something

The majority of the linked bug reports are ".gitignore should contain /target and not target", and actually asks us to ignore less, not more. Now that we have workspaces, I think we should ignore only /target, because it's usually the only target! If you use multi-package non-workspaced layout, you can use a per-package .gitignore with /target.

@matklad
Copy link
Member

matklad commented May 25, 2017

The second majority is "let's ignore backup files created by rustfmt", and the ideal solution here would be for rustfmt to not to create backup files :) @nrc any ETA on rust-lang/rustfmt#873? Looks like there are zero blocking issues: https://github.com/rust-lang-nursery/rustfmt/labels/overwrite-blocker.

@alexcrichton
Copy link
Member

@matklad hm yeah that's an excellent point! Sounds like a good idea to me.

Note that *.bk is in the gitignore right now, but if rustfmt doesn't generate them by default then I think it's fine to remove it.

@nrc
Copy link
Member

nrc commented May 25, 2017

Hmm, I guess I'm actually ready to do #873, but I think it would need a warning cycle, so maybe 6-8 weeks?

@alexcrichton
Copy link
Member

Ok I'm going to close this as it sounds like others are in general agreement for now, but thanks regardless for the PR @kpcyrd

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

Successfully merging this pull request may close these issues.

None yet

8 participants