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 Recipe that allows for the removal/negation of gitignore rules. #4610

Merged
merged 13 commits into from
Oct 28, 2024

Conversation

Jenson3210
Copy link
Contributor

What's changed?

Added a recipe

What's your motivation?

First iteration of this seems to work well.

Anyone you would like to review specifically?

@timtebeek as discussed

Have you considered any alternatives or workarounds?

FindAndReplaceText is very restrictive on this. This really uses gitignore rules/logic to determine if a change to the gitignore is necessary.

Any additional context

I've added a workaround for openrewrite/jgit#3
In a next iteration, I want to remove this workaround again once jgit is fixed but guessing we will have some longer checks there.
Planning on 2 later iterations:

  • removeEmptyGitignoreFile boolean option that removes a file if the last gitignore entry was removed or it only contains comments/empty lines after changes.
  • support wildcard filepaths or directories with wildcard as input: This should match all files matching and for each of these files calculate same negations. Instead of iteration over paths variable, we iterate over all files matching the paths wildcard variables.

Checklist

  • I've added unit tests to cover both positive and negative cases (A LOT 😄 )
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@Jenson3210
Copy link
Contributor Author

Jenson3210 commented Oct 25, 2024

Thanks @timtebeek, did not get the time for the suggestions yet today. Would've done it otherwise :)

About this build failure...
Looks like it got introduced by this suggestion not updating the parameter name but only the variable usage?
Screenshot 2024-10-25 at 14 08 15

Anyhow pushed a change that uses ctx

@timtebeek timtebeek added the recipe Requested Recipe label Oct 26, 2024
@Jenson3210
Copy link
Contributor Author

@timtebeek, I am done with final polishing.
I've refactored the accumulator to no longer need the workaround. I now decide myself per rule if I need to do pathMatch as true or false and no longer rely on the IngoreNode's internal true default value which is not always correct.

As I identified that the jgit lib is working fine when doing a TreeWalk, I considered probably in there, they do a similar thing bypassing ignoreRule.
As jgit is not really meant for recipe usage, I would not considere it a bug perse, rather tried fixing it cleaner internally.

I think i did a pretty good job (pat on the self-shoulder 😉 )

next to that I also added some logic to insert the new negating lines as close to possible with the ignoring lines.
It's still possible if comments are there that the line is splitted from the other one by a comment, but at least not all added at eof.

Copy link
Contributor

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Great to see you've worked this out; hope this helps you unignore those .jar files ahead of further changes.

@timtebeek timtebeek merged commit da715d1 into openrewrite:main Oct 28, 2024
2 checks passed
@Jenson3210 Jenson3210 deleted the gitignore_exclusions branch October 31, 2024 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
recipe Requested Recipe
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants