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 detection of generated IntelliJ files #5617

Merged
merged 5 commits into from
Nov 25, 2021
Merged

Add detection of generated IntelliJ files #5617

merged 5 commits into from
Nov 25, 2021

Conversation

shalvah
Copy link
Contributor

@shalvah shalvah commented Oct 25, 2021

Classify the .idea/ files generated by the IntelliJ IDEA family of IDEs (IDEA, PhpStorm, RubyMine, GoLand, etc) as generated.

@shalvah shalvah requested a review from a team as a code owner October 25, 2021 18:44
@github-linguist github-linguist deleted a comment from Awan27091987 Nov 3, 2021
Copy link
Collaborator

@Alhadis Alhadis left a comment

Choose a reason for hiding this comment

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

I'm not entirely sure we should be marking everything inside an .idea directory as generated. Going by GitHub's JetBrains.gitignore template and JetBrains' own VCS recommendations, it would seem that not everything is generated (and thus subject to high-churn).

Remember, generated files are hidden from diffs, meaning that important changes still warrant visibility. I'm wondering if it'd be wiser to include (^|\/)\.idea/ as vendored path as well. What do you think?

lib/linguist/generated.rb Outdated Show resolved Hide resolved
lib/linguist/generated.rb Outdated Show resolved Hide resolved
@shalvah
Copy link
Contributor Author

shalvah commented Nov 3, 2021

I'm not entirely sure we should be marking everything inside an .idea directory as generated.

Isn't it the other way around? The files are definitely generated (the IDE creates them automatically when I open a folder), but not necessarily vendored (you might check in some of them)? Or am I mixing up the definitions?

I'm thinking it's like dependency lockfiles (Gemfile.lock, package-lock.json). They can be shared, so you might include them in Git, but they are generated by the package manager.

shalvah and others added 2 commits November 3, 2021 12:44
Co-authored-by: John Gardner <gardnerjohng@gmail.com>
Co-authored-by: John Gardner <gardnerjohng@gmail.com>
@Alhadis
Copy link
Collaborator

Alhadis commented Nov 3, 2021

Isn't it the other way around? The files are definitely generated (the IDE creates them automatically when I open a folder), but not necessarily vendored (you might check in some of them)? Or am I mixing up the definitions?

No, that's correct. The key difference is that generated files don't show up in diffs on GitHub, whereas vendored/documentation files do. However, sometimes changes to machine-generated files need to be visible to reviewers (see #4348 for an example). Hence why I asked if it'd be more appropriate if we mark these files as vendored (and only the high-churn ones as generated). I should also mention I have zero knowledge of JetBrains IDEs, hence why I'm trusting your knowledge more than mine.

@shalvah
Copy link
Contributor Author

shalvah commented Nov 3, 2021

generated files don't show up in diffs on GitHub,

They do, but they're collapsed by default, yes?

Re the yarn reference: yes, that's valid, but all the .idea files hold is IDE/project settings. For instance, my workspace.xml has personal stuff like my recent commit messages, VCS settings, spellcheck settings, etc.

The files they say you can commit (real examples from one of my JS projects):

  • myproject.iml:
<module type="WEB_MODULE" version="4">
  <component name="NewModuleRootManager">
    <content url="file://$MODULE_DIR$" />
    <orderEntry type="inheritedJdk" />
    <orderEntry type="sourceFolder" forTests="false" />
  </component>
</module>
  • vcs.xml:
<project version="4">
  <component name="VcsDirectoryMappings">
    <mapping directory="$PROJECT_DIR$" vcs="Git" />
  </component>
</project>
  • misc.xml:
<project version="4">
  <component name="JavaScriptSettings">
    <option name="languageLevel" value="ES6" />
  </component>
</project>

Nice-to-haves if you want to share them, maybe, but none of these files really matter—once you open a folder in the IDE, it regenerates the whole .idea/ folder if it doesn't exist. (In fact, I haven't seen a project where they were consciously shared, though I have seen accidental check-ins from newbie devs, along with .DS_Store😅.)

There don't seem to be any malicious opportunities from hidden diffs, except messing with your teammates, and they can easily change the setting back in their IDE.

@Alhadis
Copy link
Collaborator

Alhadis commented Nov 3, 2021

In that case, this sounds good to me. 👍 Thanks for the explanation!

@lildude lildude merged commit 7c5e732 into github-linguist:master Nov 25, 2021
@shalvah shalvah deleted the intellij-detection branch December 2, 2021 11:41
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

3 participants