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

Formatter setup with checkstyle.xml doesn't honor SeparatorWrap rules in checksum #335

Open
nddipiazza opened this issue Jul 21, 2017 · 6 comments
Labels
code-style-importer Related to the code style import functionality

Comments

@nddipiazza
Copy link

I have loaded checkstyle.xml into the intellij formatter with the following separator wraps:

<module name="SeparatorWrap">
            <property name="id" value="SeparatorWrapDot"/>
            <property name="tokens" value="DOT"/>
            <property name="option" value="nl"/>
        </module>
        <module name="SeparatorWrap">
            <property name="id" value="SeparatorWrapComma"/>
            <property name="tokens" value="COMMA"/>
            <property name="option" value="EOL"/>
        </module>
        <module name="SeparatorWrap">
            <!-- ELLIPSIS is EOL until https://github.com/google/styleguide/issues/258 -->
            <property name="id" value="SeparatorWrapEllipsis"/>
            <property name="tokens" value="ELLIPSIS"/>
            <property name="option" value="EOL"/>
        </module>
        <module name="SeparatorWrap">
            <!-- ARRAY_DECLARATOR is EOL until https://github.com/google/styleguide/issues/259 -->
            <property name="id" value="SeparatorWrapArrayDeclarator"/>
            <property name="tokens" value="ARRAY_DECLARATOR"/>
            <property name="option" value="EOL"/>
        </module>
        <module name="SeparatorWrap">
            <property name="id" value="SeparatorWrapMethodRef"/>
            <property name="tokens" value="METHOD_REF"/>
            <property name="option" value="nl"/>
        </module>

yet when i format

    this.idMatcher = new IdMatcher(config.getRegexCacheSize(), diagnosticMode, properties.getIncludedFileExtensions(), properties.getExcludedFileExtensions(), properties.getIncludedFilenameRegexes(), properties.getExcludedFilenameRegexes());

it spaces it incorrectly

    this.idMatcher = new IdMatcher(config.getRegexCacheSize(), diagnosticMode, properties.getIncludedFileExtensions()
      , properties.getExcludedFileExtensions(), properties.getIncludedFilenameRegexes(), properties
      .getExcludedFilenameRegexes());

It should have respected the SeparatorWrap rule and put the comma on the previous line.

@jshiell
Copy link
Owner

jshiell commented Jul 23, 2017

Thanks for the report. I'll add it to the todo list. The formatter import was a contribution however, so I'm not familiar with the code and hence I can make no promises as to when/if this will be fixed.

@appraveen
Copy link

Please prioritize this issue. We have been struggling formatting our legacy code and it is really hard for us to do this manually.

@nddipiazza
Copy link
Author

@appraveen
Copy link

appraveen commented Sep 21, 2017

@nddipiazza thank you for the link. It will work for method parameters. There are many other instances where line wrapping has to be properly formatted

Wrap around operators

  • Multi line strings with + operator
  • Ternary operator (if condition on a single line)
  • Before '.' with method chaining (ex: new StringBuider().append().append().....

if reformat code does not take care of this, then everyone has to manually fix the line and run checkstyle to ensure all rules are passing. No wonder if we end up doing multiple tries before wrapping the line successfully.

@jshiell
Copy link
Owner

jshiell commented Sep 22, 2017

The code formatter input was a contribution, and to be honest it's not something I'm very familiar with. While there's certainly work I want to get done on the plugin, this is pretty low on the priority list and unlikely to be fixed any time soon.

If this is a major problem for you, contributions are very welcome.

@yeoji
Copy link
Contributor

yeoji commented Jan 23, 2018

I could be wrong, but it seems like IntelliJ itself does not have the option that allows us to set a wrap on commas, dots, etc in the formatter.

@jshiell jshiell added the code-style-importer Related to the code style import functionality label Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-style-importer Related to the code style import functionality
Projects
None yet
Development

No branches or pull requests

4 participants