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

support // format:off comments #137

Open
pniewiadomski-siecommerce opened this issue Mar 14, 2017 · 32 comments
Open

support // format:off comments #137

pniewiadomski-siecommerce opened this issue Mar 14, 2017 · 32 comments

Comments

@pniewiadomski-siecommerce
Copy link

pniewiadomski-siecommerce commented Mar 14, 2017

Currently formatter ignores //@formatter:off and :on tags.

@jbduncan
Copy link
Contributor

jbduncan commented Mar 14, 2017

Currently formatter ignores //@formatter:off and :on tags.

Which is understandable since it explicitly says in the README that,

Note: There is no configurability as to the formatter's algorithm for formatting. This is a deliberate design decision to unify our code formatting on a single format.

So I wouldn't be surprised if the google-java-format team rejected this idea.

@pniewiadomski-siecommerce
Copy link
Author

Hi, by configurbility I understand that there are no specific options to be set like indentation or others. That is completely understandable. If GJF was configurable it would became Custom Java Format.
However, the mentioned tags are, imo, outside of this scope, hence they are just an instruction to switch formatter completely off at some places.

@lowasser
Copy link
Contributor

I don't think we have it in the external version, but our internal version has an FAQ entry:

Why can't I use magic comments to disable formatting for certain sections of code?

We realize this situation will occasionally be annoying—but is it is annoying enough, often enough, to justify peppering our codebase with special // format:off comments?

At the present time, the jury is out (bug link). The annoyance you feel may be transitory, while those formatting comments would persist, visibly, in the code for everyone to see forever. They would reduce the signal-to-noise ratio and hurt readability, running counter to our goals.

I'm not sure if we consider the external version differently.

@cushon cushon changed the title [feature] Make eclipse plugin version of formatter respect on/off tags support // format:off comments Oct 14, 2017
@jmmk
Copy link

jmmk commented Oct 23, 2017

@lowasser The other side of that argument is

Is not having support for @formatter:off annoying enough, often enough, to justify including the feature?

The reluctance you feel may be transitory, while those poorly-formatted sections of code will persist, visible, in the code for everyone to be confused by forever. They hurt readability, running counter to our goals

Where I've used @formatter:off it's because indentation corresponds to some semantic meaning, either in a DSL or in a large block comment that's structured with specific spacing.

I realize it's a little bit of a slippery slope - the point of an opinionated formatter is to remove the need to make any decisions, so why introduce a decision point? I think the fact that this is a single on-off switch make it less worrying, but like anything it does have the potential to be abused.

@sormuras
Copy link
Contributor

sormuras commented Nov 5, 2018

Google Java Format does not format XML files.

@tbroyer
Copy link
Contributor

tbroyer commented Nov 5, 2018

@alessandrococco Fwiw, have you tried putting xml:space="preserve" on the configurationParameters to tell the XML formatter not to reformat that section? (https://www.w3.org/TR/xml/#sec-white-space)

@alessandrococco
Copy link

@tbroyer It works, thanks! I'm happy to have learned something new :-)

@cushon
Copy link
Collaborator

cushon commented Nov 5, 2018

FWIW our thinking on how this applies to Java code hasn't really changed since @lowasser's comment: we want to do what we can to avoid the need for // format:off comments, and the jury is still out about supporting them in the future.

If anyone has specific examples of common patterns that would benefit from this, that's be useful information.

@jmmk mentioned:

either in a DSL or in a large block comment that's structured with specific spacing

Note that we don't re-format /* ... */ block comments. For javadoc comments that shouldn't be reformatted, you can use <pre>...</pre> to ensure whitespace is preserved.

@mbruce
Copy link

mbruce commented Nov 6, 2018

Here is a specific example. There are many many of these, and thinking that a formatter can solve all problems everywhere is a bad idea. In this case I have a matrix that is 4x4. I would like to format it to look like a 4x4 matrix, since that is clearly more readable:

public static final int[] defaultScalingList4x4Inter = {
    10, 14, 14, 20, 
    20, 20, 24, 24, 
    24, 24, 27, 27, 
    27, 30, 30, 34
  };

Google format does this instead:

public static final int[] defaultScalingList4x4Intra = {
    6, 13, 13, 20, 20, 20, 28, 28, 28, 28, 32, 32, 32, 37, 37, 42
};

@tbroyer
Copy link
Contributor

tbroyer commented Nov 6, 2018

@mbruce Put // at the end of your lines to disable line joining:

  public static final int[] defaultScalingList4x4Inter = {
      10, 14, 14, 20, //
      20, 20, 24, 24, //
      24, 24, 27, 27, //
      27, 30, 30, 34
  };

@cushon
Copy link
Collaborator

cushon commented Nov 6, 2018

The // hack is unnecessary here. The formatter preserves 'tabular' array initializers:

$ cat T.java
class T {
  public static final int[] defaultScalingList4x4Inter = {
    10, 14, 14, 20,
    20, 20, 24, 24,
    24, 24, 27, 27,
    27, 30, 30, 34
  };
}
$ google-java-format T.java
class T {
  public static final int[] defaultScalingList4x4Inter = {
    10, 14, 14, 20,
    20, 20, 24, 24,
    24, 24, 27, 27,
    27, 30, 30, 34
  };
}

If the input isn't already tabular it won't reformat into a grid. But this is one of the only places it recognizes the existing formatting.

@cushon
Copy link
Collaborator

cushon commented Dec 19, 2019

@lowasser's comment is now captured in this FAQ entry.

This isn't something we have plans to revisit right now, but it's always helpful to see specific examples--if you have snippets of code where you wish you could use a // format:off comment, please feel free to share them.

@kevin-lin
Copy link

when using a builder object, I am running into an issue where the formatter is adding line breaks to long lines. This is making things hard to read because arguments no longer align vertically and I am unable to visually skim my code. Here's an example:

Desired format:

builder
    .NodeToNodeEdge(Node.SOME_ENUM, Node.SOME_ENUM_A)
    .NodeToNodeEdge(Node.SOME_ENUM, Node.SOME_LONG_ENUM_THAT_MAKES_LINE_TOO_LONG_B)
    .NodeToNodeEdge(Node.SOME_ENUM, Node.SOME_ENUM_C)
    .NodeToNodeEdge(Node.SOME_ENUM, Node.SOME_ENUM_D)
    .NodeToNodeEdge(Node.SOME_ENUM, Node.SOME_LONG_ENUM_THAT_MAKES_LINE_TOO_LONG_E)
    .NodeToNodeEdge(Node.SOME_ENUM, Node.SOME_ENUM_F)
    .build();

Formatted code:

builder
    .NodeToNodeEdge(Node.SOME_ENUM, Node.SOME_ENUM_A)
    .NodeToNodeEdge(
        Node.SOME_ENUM, Node.SOME_LONG_ENUM_THAT_MAKES_LINE_TOO_LONG_B)
    .NodeToNodeEdge(Node.SOME_ENUM, Node.SOME_ENUM_C)
    .NodeToNodeEdge(Node.SOME_ENUM, Node.SOME_ENUM_D)
    .NodeToNodeEdge(
        Node.SOME_ENUM, Node.SOME_LONG_ENUM_THAT_MAKES_LINE_TOO_LONG_E)
    .NodeToNodeEdge(Node.SOME_ENUM, Node.SOME_ENUM_F)
    .build();

@TimvdLippe
Copy link
Contributor

I just had a test failure after running google-java-format on Mockito source code. The test in question tests some behavior if 2 statements are on the same line. The formatter then puts them on separate lines, which in turn makes the test fail.

I am going to ignore formatting of the whole file in Spotless, but a code comment would have been ideal here.

@rauf
Copy link

rauf commented Jul 19, 2020

I am running into the issue where the formatter is indenting the lines to the same depth. This is making code harder to read. Spring Security uses builder pattern heavily, so indentation plays an important role in code comprehension.

Desired code:

        http.
                cors()
                    .and()
                .sessionManagement()
                    .sessionCreationPolicy(SessionCreationPolicy.STATELESS)
                    .and()
                .csrf()
                    .disable()
                .formLogin()
                    .disable()
                .httpBasic()
                    .disable()
                .exceptionHandling()
                    .authenticationEntryPoint(new RestAuthenticationEntryPoint())
                    .and()
                .authorizeRequests()
                    .mvcMatchers(
                            "/error",
                            "/api")
                        .permitAll()
                    .antMatchers("/auth/**")
                        .permitAll()
                    .anyRequest()
                        .authenticated()
                    .and()
                .oauth2Login()
                  // more lines

Formatted code:

    http.cors()
        .and()
        .sessionManagement()
        .sessionCreationPolicy(SessionCreationPolicy.STATELESS)
        .and()
        .csrf()
        .disable()
        .formLogin()
        .disable()
        .httpBasic()
        .disable()
        .exceptionHandling()
        .authenticationEntryPoint(new RestAuthenticationEntryPoint())
        .and()
        .authorizeRequests()
        .mvcMatchers("/error", "/api")
        .permitAll()
        .antMatchers("/auth/**")
        .permitAll()
        .anyRequest()
        .authenticated()
        .and()
        .oauth2Login()

@nedtwigg
Copy link
Contributor

nedtwigg commented Sep 11, 2020

If you're using google-java-format through Spotless, we just added support for on/off tags in plugin-gradle 5.5.0 (like this) and plugin-maven 2.3.0 (like this).

@richardstephens
Copy link

Here is a use-case I don't see mentioned elsewhere in this thread.

We use a static-analysis tool that uses comments to disable rules for a specific line. However, it requires the comments to be formatted as //RULE. Google Java format inserts a space between the // and RULE, causing the comment not to be picked up by the tool. In this case using a //formatter:off and //formatter:on comments would allow us to keep using the formatter for the rest of the file.

@nedtwigg
Copy link
Contributor

nedtwigg commented Dec 7, 2020

You could fix that in spotless (gradle or maven) with:

spotless {
  googleJavaFormat()
  replace 'fix static-analysis', '// RULE', '//RULE'
}

or you can use formatter:off blocks as per my comment above. It's not necessary for gjf to handle all use-cases if you are using a system that allows you to compose big complicated formatters like google-java-format with smaller find-replace tools.

@ankurkgupta
Copy link

This could be useful in excluding GWT's JSNI method formatting

@lgylym
Copy link

lgylym commented Feb 25, 2022

One more example I want to add is when we are using k8s V1DeploymentBuilder to build k8s object, it would be really helpful to have custom indent to show the nested relationship instead of the following style (which in reality has hundreds of lines):

    return new V1DeploymentBuilder()
        .withApiVersion("apps/v1")
        .withKind("Deployment")
        .withNewMetadata()
        .withNamespace("example")
        .withName("example-deployment")
        .addToLabels("name", "value")
        .endMetadata()
        .withNewSpec()
        .withReplicas(1)
        .withNewSelector()
        .withMatchLabels(Collections.singletonMap("app", "test-deployment"))
        .endSelector()
        .withNewTemplate()
        .withNewMetadata()
        .addToLabels("app", "test-deployment")
        .endMetadata()
        .withNewSpec()
        .withContainers()
        .addNewContainer()
        .withName("hello-world")
        .withImage("gcr.io/example-image")
        .addNewPort()
        .withName("http")
        .withContainerPort(8080)
        .endPort()
        .withNewResources()
        .addToRequests("cpu", Quantity.fromString("200m"))
        .addToRequests("memory", Quantity.fromString("200m"))
        .addToLimits("cpu", Quantity.fromString("500m"))
        .addToLimits("memory", Quantity.fromString("1G"))
        .endResources()
        .endContainer()
        .endSpec()
        .endTemplate()
        .endSpec()
        .build();
  }

@eaball35
Copy link

eaball35 commented Mar 1, 2022

I'm working on adding region tags (go/code-snippets-101#region-tag) to generated java sample code. The region tag comments are required to live on their own line. The code formatter splits long comments over multiple lines and moves the final end tag comment up a line. //formatter:off would allow me to not have to do post processing after formatting sample code.

@zg-trinac
Copy link

zg-trinac commented Aug 24, 2022

+1 to the depth issue called out previously. My case is using GraphQL in Java via DGS generated code, where the indentation helps show the children vs parent relationship of the query. This is an example from their docs, but I have a use case where I'm running into the same issue.

Desired result:

GraphQLQueryRequest graphQLQueryRequest =
        new GraphQLQueryRequest(
            new TicksGraphQLQuery.Builder()
                .first(first)
                .after(after)
                .build(),
            // Indenting to show children
            // @formatter:off
            new TicksConnectionProjection()
                .edges()
                    .node()
                        .date()
                        .route()
                            .name()
                            .votes()
                                .starRating()
                                .parent()
                            .grade());
            // @formatter:on

Each method in the Projection is walking through the query tree (e.g. Parent to child, route > votes > starRating)

@jwwallin
Copy link

jwwallin commented Oct 18, 2022

I don't see Apache Camel mentioned yet. But it falls in the "DSL/indentation" category.

Camel has a Java DSL. The DSL code can quite easily become so complex that formatting is essential to being able to understand what is happening. It seems like spotless might be able to handle this situation, but I need to test it.

Edit: Spotless can indeed take care of this need.

@oxygenecore
Copy link

Another example I occasionally have is table-like formatting inside of java code:

    double[] rowname =       {22147.51,  15249.58,   42515.75,   48948.44,   137977.49,  1075.66,  132302.42,    512.42,   };
    double[] small =         {4.64,      4.20,       9.74,       14.09,      37.68,      0.29,     38.92,        0.15,     };
    double[] biggggcolname = {0.74,      0.67,       1.52,       2.25,       6.02,       0.00,     0.00,         0.00,     };
    double[] onemore =       {4.64,      4.20,       9.51,       14.09,      37.68,      0.00,     0.00,         0.00,     };

@kberezin-nshl
Copy link

kberezin-nshl commented Jun 23, 2023

Elasticsearch API. What I would like to have:

var hits =
          elasticsearchClient
              .search(r -> r.index(indexName)
                  .query(q -> q.bool(
                      b -> b
                          .must(job -> job.match(t -> t.field("job_id").query(jobId)))
                          .must(str -> str.queryString(
                              qs -> qs.query("\"%s\"".formatted(searchString))))))
                  .size(request.getPageSize()),
                  Doc.class);

vs what I get

      var hits =
          elasticsearchClient
              .search(
                  r ->
                      r.index(indexName)
                          .query(
                              q ->
                                  q.bool(
                                      b ->
                                          b.must(
                                                  job ->
                                                      job.match(
                                                          t -> t.field("job_id").query(jobId)))
                                              .must(
                                                  str ->
                                                      str.queryString(
                                                          qs ->
                                                              qs.query(
                                                                  "\"%s\""
                                                                      .formatted(searchString))))))
                          .size(request.getPageSize()),
                  Doc.class);

overall Java's lambda support is incredibly poor. Why don't you just support Intellij's already supported feature: turning formatter off with a comment, @cushon ?

@alshopov
Copy link

alshopov commented Jul 4, 2023

This ticket was created in 2017 and there has been no change no matter what examples people give. Can this be added as a WONT_DO_ON_PURPOSE in the FAQ? Basically teams should be well aware of what they are buying in.

It is ok to leave the ticket open - then whoever wants to complain has to go through this ticket which enables easier mail filtering.

In many places the person doing a code change has no possibility of opting out of a file from the formatter as it would be a different team imposing build/formatting standards. The fact people bother complaining here may be just a symptom they believe it is futile complaining elsewhere.

There is no stop to this end, I am torn what to call this:
the Eternal enjambment or Formatter concrete.

Just for completeness - it is also impossible to affect the layout and draw their own code calligram via creative usage of empty comments at end of line or in between the tokens.

On the one hand - it really irks me that a small change can trigger a full reflow. On the other hand - perhaps a change that reflows is not worth it and can provide a good incentive to people to not do changes. On the third hand - people get used to Lisp-y code layout so there is my silver lining.

@tbroyer
Copy link
Contributor

tbroyer commented Jul 4, 2023

Well, it's already in the FAQ: https://github.com/google/google-java-format/wiki/FAQ#why-cant-i-use-magic-comments-to-disable-formatting-for-certain-sections-of-code

(as has been linked above 3 years ago: #137 (comment))

@alshopov
Copy link

alshopov commented Jul 4, 2023

Well, it's already in the FAQ:...
(as has been linked above 3 years ago: #137 (comment))

Indeed you are right - my bad. I must have been blind with rage.

A re-format forces different attribution when doing git/svn annotate and makes history checks harder. It also makes impossible separating a semantic change from a reformatting as all must be mushed together.

Yep, the annoyance is transient - it will not be serialized in the code. If a small change forces a reflow it must be a worthwhile change. Not every change is worth a reflow.

@Marcono1234
Copy link

Another use case: When you want to make sure a multi line string is split across separate lines, especially useful for unit tests which want to verify certain formatted output.

String json = 
    "{\n"
    + "  \"a\": [\n"
    + "    1,\n"
    + "    2,\n"
    + "    3,\n"
    + "    4\n"
    + "  ]\n"
    + "}";

Is changed by Google Java Format to:

String json =
    "{\n" + "  \"a\": [\n" + "    1,\n" + "    2,\n" + "    3,\n" + "    4\n" + "  ]\n" + "}";

(it appears it only keeps the separate lines when the single line variant would reach the maximum line length)

@ZeyuKeithFu
Copy link

Is there any workaround to disable formatting for an import block?

import D.E.F;
// Grouped import
import A.B.C;
// Grouped import

The reason I am doing this is that, I want to separate my imports from upstream code and reduce merge conflicts when pulling upstream codes. Formatter will always mix them together and became:

import A.B.C;
import D.E.F;
// Grouped import
// Grouped import

@tbroyer
Copy link
Contributor

tbroyer commented Jan 12, 2024

@ZeyuKeithFu In this specific case, maybe --skip-sorting-imports would be enough?

@ZeyuKeithFu
Copy link

@ZeyuKeithFu In this specific case, maybe --skip-sorting-imports would be enough?

Thanks for the hint. Yes, it would be enough for this single file. The problem remains when formatting multiple files. I still want to make sure files without "grouped imports" are sorting imports correctly.

Also, the imports outside of "grouped imports" will not be sorted.

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

No branches or pull requests