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

Indent with spaces rather than with tabs #4202

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mernst
Copy link
Contributor

@mernst mernst commented Nov 12, 2023

The JavaParser codebase mainly uses spaces for indentation. However, in a few places it inconsistently uses tabs. This pull request changes tabs to spaces in .java files.

Copy link

codecov bot commented Nov 12, 2023

Codecov Report

Attention: Patch coverage is 75.55147% with 133 lines in your changes are missing coverage. Please review.

Project coverage is 57.993%. Comparing base (6726113) to head (f4b47f8).
Report is 184 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff               @@
##              master     #4202       +/-   ##
===============================================
- Coverage     57.996%   57.993%   -0.003%     
  Complexity      2334      2334               
===============================================
  Files            645       645               
  Lines          34578     34578               
  Branches        5976      5976               
===============================================
- Hits           20054     20053        -1     
  Misses         12366     12366               
- Partials        2158      2159        +1     
Flag Coverage Δ
AlsoSlowTests 57.993% <75.551%> (-0.003%) ⬇️
javaparser-core 50.526% <51.724%> (-0.004%) ⬇️
javaparser-symbol-solver 37.087% <39.338%> (ø)
jdk-10 57.981% <75.000%> (-0.003%) ⬇️
jdk-11 57.981% <75.000%> (-0.003%) ⬇️
jdk-12 57.981% <75.000%> (-0.003%) ⬇️
jdk-13 57.981% <75.000%> (-0.003%) ⬇️
jdk-14 57.981% <75.000%> (-0.003%) ⬇️
jdk-15 57.981% <75.000%> (-0.003%) ⬇️
jdk-16 57.950% <73.345%> (-0.003%) ⬇️
jdk-8 57.978% <74.816%> (-0.003%) ⬇️
jdk-9 57.981% <75.000%> (-0.003%) ⬇️
macos-latest 57.987% <75.551%> (-0.003%) ⬇️
ubuntu-latest 57.981% <75.551%> (-0.003%) ⬇️
windows-latest 57.976% <75.551%> (-0.003%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ithub/javaparser/ast/body/CallableDeclaration.java 59.302% <ø> (ø)
...parser/ast/body/CompactConstructorDeclaration.java 29.710% <ø> (ø)
...m/github/javaparser/ast/comments/BlockComment.java 73.684% <100.000%> (ø)
...va/com/github/javaparser/ast/comments/Comment.java 58.490% <100.000%> (ø)
...github/javaparser/ast/comments/JavadocComment.java 75.000% <100.000%> (ø)
...om/github/javaparser/ast/comments/LineComment.java 78.947% <100.000%> (ø)
...ub/javaparser/ast/observer/ObservableProperty.java 88.757% <100.000%> (ø)
...java/com/github/javaparser/ast/type/ArrayType.java 89.130% <100.000%> (ø)
...thub/javaparser/ast/type/ClassOrInterfaceType.java 70.642% <100.000%> (ø)
.../com/github/javaparser/ast/type/PrimitiveType.java 77.611% <100.000%> (ø)
... and 75 more

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6726113...f4b47f8. Read the comment docs.

@hazendaz
Copy link
Contributor

Hi @mernst, given this is so large of a change and impact it can have on existing pulls, maybe it can be approached in a separate way.

Here are my thoughts on it.

  • First run com.github.hazendaz.maven:whitespace-maven-plugin:1.3.0:trim as that one is less intrusive (plugin I maintain) and I saw some trailing spacing in various files I've been in. I think this is less likely to cause issues with open PRs.
  • I would break this up into change per module only so its not so large but do those back to back until done.

Just my food for thought, and I agree tags in any files are bad. Not very cross platform nice and its getting mixed by contributors is pretty frequent. If you don't already you may want to make sure to have plugin do this automatically after done so it doesn't happen again. Spotless maven plugin is probably best candidate to cover the files in fast way.

@hazendaz
Copy link
Contributor

There are also some tricks, I've seen 'maven' team doing where they make git ignore the commits involved since they add little value and can be seen as a lot of noise. Not sure that is worth the effort or concern as one can ignore whitespace and this is same line stuff but worth noting that is possible with git.

@mernst
Copy link
Contributor Author

mernst commented Nov 27, 2023

I agree that use of Spotless is better than trying to manually enforce the style. It can also "ratchet": enforce formatting only for lines that have changed, so there are no irrelevant changes in the version control history. I'll leave applying the trim plugin and/or Spotless up to you.

@jlerbsc jlerbsc added the Improvement Not a bug, but a way that JP can be be enhanced to work better. label Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Not a bug, but a way that JP can be be enhanced to work better.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants