Whitespace and formatting in Intellij

[Toby Reyelts]

On Jan 29, 2008 12:44 PM, Scott Blum <sco...@google.com> wrote:

Hi Toby,

Some formatting questions:

- Can IntelliJ be configured not to break on the 'dot' operator?  In JavaToJavaScriptCompiler(305) for example, this would always be formatted such that the entirety of "GenerateJavaAST.exec(allTypeDeclarations," would never be broken; the first break would happen after the first parameter.

I'm not sure why that was that way. I fixed the line break and did an auto-format, and it didn't introduce a new break. /shrug

- Whitespace at end of lines: is this an IntelliJ issue, or were the new sections of EnumsTest (for example) not reformatted?  An Eclipse autoformat would leave lines 200 and 255 blank, and truncate line 217 for example.  This is a lot more apparently when using a diff tool/editor set to show whitespace characters.  (I saw this in other files as well.)

There are three options:

1) Strip all trailing whitespace
2) Only strip trailing whitespace for lines that you modify
3) Never strip trailing whitespace
 
The first obviously won't work, since eclipse doesn't have that functionality. The second is really a pain, because Intellij "remembers" which lines you've modified. The third ends up saving trailing whitespace that you don't seem to like.

I've really tried to give an effort to meet our eclipse's settings as much as possible, but there are 260 settings in the eclipse code formatting styles - and that doesn't include the large set of CodeStyle rules or the 24 rules I had to set up for ordering. In some cases obvious things are easily met, and in other cases (such as whitespace), I'm not sure that there is any solution where Intellij can act as a perfect eclipse emulator. Can we not come to an agreement that is amenable to more editors/IDEs than eclipse alone?

Thanks,
-Toby

--Scott

On Jan 28, 2008 6:07 PM, Toby Reyelts <to...@google.com> wrote:

Hi Scott (and all),

Would you please be kind enough to review this patch that updates our JRE emul classes such that they can be compiled by Sun's JDK (tested with 1.5.0._07). From issue 2039:

Problems:

1) Call sites where javac is not inferencing type parameters of generic
functions where the type parameter is only used for the return type.

2) javac explicitly uses Class.desiredAssertionStatus when it compiles
assert statements.

3) javac explicitly uses Enum.valueOf(Class,String) when it encounters enums.

The attached patch explicitly specifies type parameters where needed and
implements both Class.desiredAssertionStatus and Enum.valueOf.

Thanks,
-Toby

[Scott Blum]

Everyone,

I'd really like to get some feedback from a number of people on some of these issues.  To put more perspective on it, we've been going around on these issues for a while, but it seems like we've hit the limit on the extent to which we can make Eclipse and IntelliJ agree to autoformat the same way.

Maybe it's time to have a larger discussion and rethink the original purposes.  How important is auto-format?  Is it important to not thrash whitespace in svn?  Should we make everyone use Eclipse (at least for formatting) or perhaps dive into the Eclipse code and hack together a command line tool to perform Eclipse auto-formatting?

Thoughts?
Scott

[Fred Sauer]

On Jan 29, 2008 1:55 PM, Toby Reyelts <to...@google.com> wrote:

On Jan 29, 2008 12:44 PM, Scott Blum <sco...@google.com> wrote:

Hi Toby,

Some formatting questions:

- Can IntelliJ be configured not to break on the 'dot' operator?  In JavaToJavaScriptCompiler(305) for example, this would always be formatted such that the entirety of "GenerateJavaAST.exec(allTypeDeclarations," would never be broken; the first break would happen after the first parameter.

I'm not sure why that was that way. I fixed the line break and did an auto-format, and it didn't introduce a new break. /shrug

- Whitespace at end of lines: is this an IntelliJ issue, or were the new sections of EnumsTest (for example) not reformatted?  An Eclipse autoformat would leave lines 200 and 255 blank, and truncate line 217 for example.  This is a lot more apparently when using a diff tool/editor set to show whitespace characters.  (I saw this in other files as well.)

There are three options:

1) Strip all trailing whitespace
2) Only strip trailing whitespace for lines that you modify
3) Never strip trailing whitespace
 
The first obviously won't work, since eclipse doesn't have that functionality. The second is really a pain, because Intellij "remembers" which lines you've modified. The third ends up saving trailing whitespace that you don't seem to like.

Eclipse has this under Clean Up, which can be initiated manually, or automatically when you save.

  Preferences -> Java -> Code Style -> Clean Up -> Code Organizing -> Formatter -> Remove Trailing whitespace -> All Lines




 

I've really tried to give an effort to meet our eclipse's settings as much as possible, but there are 260 settings in the eclipse code formatting styles - and that doesn't include the large set of CodeStyle rules or the 24 rules I had to set up for ordering. In some cases obvious things are easily met, and in other cases (such as whitespace), I'm not sure that there is any solution where Intellij can act as a perfect eclipse emulator. Can we not come to an agreement that is amenable to more editors/IDEs than eclipse alone?

Thanks,
-Toby

--Scott

On Jan 28, 2008 6:07 PM, Toby Reyelts <to...@google.com> wrote:

Hi Scott (and all),

Would you please be kind enough to review this patch that updates our JRE emul classes such that they can be compiled by Sun's JDK (tested with 1.5.0._07). From issue 2039:

Problems:

1) Call sites where javac is not inferencing type parameters of generic
functions where the type parameter is only used for the return type.

2) javac explicitly uses Class.desiredAssertionStatus when it compiles
assert statements.

3) javac explicitly uses Enum.valueOf(Class,String) when it encounters enums.

The attached patch explicitly specifies type parameters where needed and
implements both Class.desiredAssertionStatus and Enum.valueOf.

Thanks,
-Toby

--
Fred Sauer
fr...@allen-sauer.com

[Fred Sauer]

Personally, I think getting auto format to work consistently is very important for a couple of reasons:

1. Remove or at least minimize an otherwise major distraction for code reviews
2. Tremendous time savings when working on patches since you can auto format the entire file and know that only modified code will be reformatted
   

Once good, ideally cross-IDE, settings have been determined, I think it would be worth a "global reformat and commit everything" so that going forward non-auto-formatted code doesn't get in the way.


On a practical note: as external contributors, we don't have access to a JSNI formatting plugin, which means that no matter how we configure our IDE, we cannot possibly get correctly formatted code in an automated fashion. I have, however, found a partial work-around in Eclipse, which is to turn off formatting of comments, and to move the opening JSNI fragment to the line following the method name. If you do this, you can at least auto format everything but the comments.

So, while Eclipse will mess up the indentation on this...
  native void foo() /*-{
  }-*/;

...it will leave the JSNI code alone if your turn off auto formatting of comments and instead use this...
  native void foo()
  /*-{
  }-*/;

Fred

--
Fred Sauer
fr...@allen-sauer.com

[Eric Ayers]

Since we are so diligent about code reviews, having whitespace screw up a diff is big deal IMHO.

Does the whitespace normalization tool even have to exactly match the Eclipse settings?  So long as it is a consistent formatter, we could just add it as an 'ant' target that any contributor (Eclipse user or otherwise) runs before creating a patch or committing.   It doesn't seem likely we'll be able to keep the took up to date with Eclipse's formatter over the long haul.

And to pile on, what about the Sort methods... function?  I'm continuously being pinged for forgetting to use it, although I'm sure I'll eventually catch on.

Speaking of which, is there a more convenient way to run auto format on a patch than to pull up each file and run Source->Format (or the kbd shortcut?)   For a patch with a lot of files, this is tedious. 


-Eric.

--
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

[Fred Sauer]

On my own personal projects I setup project settings 'Java Code Style -> Clean Up' and 'Java Editor -> Save Actions' (especially 'Additional Actions') to fairly aggressively format my files. In (really) minor projects I sometimes even force an auto sort of all members. This can of course be dangerous, but by using initializer blocks, I can still ensure a consistent initialization order of members fields.

Personally I do all code editing from the IDE, although I tend to apply/create patches at the command line, but that's just a personal style.

Fred

--
Fred Sauer
fr...@allen-sauer.com

[Toby Reyelts]

On Jan 29, 2008 4:16 PM, Fred Sauer <fr...@allen-sauer.com> wrote:

On Jan 29, 2008 1:55 PM, Toby Reyelts <to...@google.com> wrote:

On Jan 29, 2008 12:44 PM, Scott Blum <sco...@google.com> wrote:

Hi Toby,

Some formatting questions:

- Can IntelliJ be configured not to break on the 'dot' operator?  In JavaToJavaScriptCompiler(305) for example, this would always be formatted such that the entirety of "GenerateJavaAST.exec(allTypeDeclarations," would never be broken; the first break would happen after the first parameter.

I'm not sure why that was that way. I fixed the line break and did an auto-format, and it didn't introduce a new break. /shrug

- Whitespace at end of lines: is this an IntelliJ issue, or were the new sections of EnumsTest (for example) not reformatted?  An Eclipse autoformat would leave lines 200 and 255 blank, and truncate line 217 for example.  This is a lot more apparently when using a diff tool/editor set to show whitespace characters.  (I saw this in other files as well.)

There are three options:

1) Strip all trailing whitespace
2) Only strip trailing whitespace for lines that you modify
3) Never strip trailing whitespace
 
The first obviously won't work, since eclipse doesn't have that functionality. The second is really a pain, because Intellij "remembers" which lines you've modified. The third ends up saving trailing whitespace that you don't seem to like.

Eclipse has this under Clean Up, which can be initiated manually, or automatically when you save.

  Preferences -> Java -> Code Style -> Clean Up -> Code Organizing -> Formatter -> Remove Trailing whitespace -> All Lines

Fred, this would be a good way to get the same behavior, but it doesn't seem to work on comments.

[Fred Sauer]

Well, that's odd and unfortunate. I guess I never realized this because I have auto formatting of javadoc comments turned off (so I don't loose by JSNI indentation).

--
Fred Sauer
fr...@allen-sauer.com

[Miguel Méndez]

On Jan 29, 2008 4:12 PM, Scott Blum <sco...@google.com> wrote:

Everyone,

I'd really like to get some feedback from a number of people on some of these issues.  To put more perspective on it, we've been going around on these issues for a while, but it seems like we've hit the limit on the extent to which we can make Eclipse and IntelliJ agree to autoformat the same way.

Maybe it's time to have a larger discussion and rethink the original purposes.  How important is auto-format?  Is it important to not thrash whitespace in svn?  Should we make everyone use Eclipse (at least for formatting) or perhaps dive into the Eclipse code and hack together a command line tool to perform Eclipse auto-formatting?

FWIW, I believe that you can already launch the Eclipse formatter application from the command line.  See: http://help.eclipse.org/help32/index.jsp?topic=/org.eclipse.jdt.doc.user/tasks/tasks-232.htm
 

Thoughts?
Scott

On Jan 29, 2008 3:55 PM, Toby Reyelts <to...@google.com> wrote:

On Jan 29, 2008 12:44 PM, Scott Blum <sco...@google.com> wrote:

Hi Toby,

Some formatting questions:

- Can IntelliJ be configured not to break on the 'dot' operator?  In JavaToJavaScriptCompiler(305) for example, this would always be formatted such that the entirety of "GenerateJavaAST.exec(allTypeDeclarations," would never be broken; the first break would happen after the first parameter.

I'm not sure why that was that way. I fixed the line break and did an auto-format, and it didn't introduce a new break. /shrug

- Whitespace at end of lines: is this an IntelliJ issue, or were the new sections of EnumsTest (for example) not reformatted?  An Eclipse autoformat would leave lines 200 and 255 blank, and truncate line 217 for example.  This is a lot more apparently when using a diff tool/editor set to show whitespace characters.  (I saw this in other files as well.)

There are three options:

1) Strip all trailing whitespace
2) Only strip trailing whitespace for lines that you modify
3) Never strip trailing whitespace
 
The first obviously won't work, since eclipse doesn't have that functionality. The second is really a pain, because Intellij "remembers" which lines you've modified. The third ends up saving trailing whitespace that you don't seem to like.

I've really tried to give an effort to meet our eclipse's settings as much as possible, but there are 260 settings in the eclipse code formatting styles - and that doesn't include the large set of CodeStyle rules or the 24 rules I had to set up for ordering. In some cases obvious things are easily met, and in other cases (such as whitespace), I'm not sure that there is any solution where Intellij can act as a perfect eclipse emulator. Can we not come to an agreement that is amenable to more editors/IDEs than eclipse alone?

Thanks,
-Toby

--
Miguel

[Fred Sauer]

Perhaps a checkstyle rule to catch trailing comment whitespace would be of assistance?


--
Fred Sauer
fr...@allen-sauer.com

[Scott Blum]

On Jan 30, 2008 8:44 AM, Miguel Méndez <mme...@google.com> wrote:

FWIW, I believe that you can already launch the Eclipse formatter application from the command line.  See: http://help.eclipse.org/help32/index.jsp?topic=/org.eclipse.jdt.doc.user/tasks/tasks-232.htm

Miguel, you rock.  I've attached a formatter config file to this email, but let's see about getting a tool into source control.

Scott

[Lex Spoon]

The Eclipse strategy looks good, and attached is a patch that adds a "format" target to every build.xml that currently has a "checkstyle" target.  It basically works, but there are two limitations.

First, it requires that you have an existing Eclipse installation somewhere.  You must either have "eclipse" on your PATH, or you must set the $GWT_ECLIPSE environment variable.  It looks hard to disentangle the Eclipse formatter from Eclipse.

Second, the formatter does go into JSNI methods.  I am posting it as is, in the hopes that some Eclipse guru could figure out how to make the formatting rules do the right thing for JSNI methods.  Alternatively, we might decide that the auto-formatted JSNI is reasonable.


On a different strategy, I also spent some time looking into alternative code formatters that could be used from the command line, but I did not find anything that is good, open source, and not wedded to an IDE.  The best two I found were Jalopy and jpplib.  The open-source version of Jalopy is not actively maintained, and jpplib appears to be minimal so I did not look at it much.  Either route, though, would mean avoiding a dependency on Eclipse.


-Lex

[John Tamplin]

On Feb 4, 2008 4:32 PM, Lex Spoon <sp...@google.com> wrote:

Second, the formatter does go into JSNI methods.  I am posting it as is, in the hopes that some Eclipse guru could figure out how to make the formatting rules do the right thing for JSNI methods.  Alternatively, we might decide that the auto-formatted JSNI is reasonable.

I don't think what Eclipse currently does with JSNI is acceptable.  I think we can figure out the internals, override necessary parts with our own classes in the classpath ahead of the Eclipse jars, and correct the behavior there, but that will be a lot of work and fragile to Eclipse version differences.  Maybe once we have a plugin that properly formats JSNI, that plugin could be hooked into this command-line formatter tool.

--
John A. Tamplin
Software Engineer, Google

[Lex Spoon]

On Feb 4, 2008 6:30 PM, John Tamplin <j...@google.com> wrote:

On Feb 4, 2008 4:32 PM, Lex Spoon <sp...@google.com> wrote:

Second, the formatter does go into JSNI methods.  I am posting it as is, in the hopes that some Eclipse guru could figure out how to make the formatting rules do the right thing for JSNI methods.  Alternatively, we might decide that the auto-formatted JSNI is reasonable.

I don't think what Eclipse currently does with JSNI is acceptable.

Okay.

 

  I think we can figure out the internals, override necessary parts with our own classes in the classpath ahead of the Eclipse jars, and correct the behavior there, but that will be a lot of work and fragile to Eclipse version differences.  Maybe once we have a plugin that properly formats JSNI, that plugin could be hooked into this command-line formatter tool.

If we are going to write some code, there are two other paths to consider.

One is to revive Jalopy.  Jalopy is a much smaller dependency than Eclipse, small enough that we could check the jars into the repository.  There is also an old Eclipse plugin for Jalopy, and that could also be revived if anyone was motivated enough. 

Two would be to write enough code that we only need the JDT and not all of Eclipse in order to run a code format.  We already check a lot of the JDT into the repository anyway, so it would not be a big deal to check in a slightly modified version.  The modification might use classpath overriding if that works best.


I am going back and forth about which way sounds better.  It depends on whether Jalopy is close enough where it is (maybe it sucks, I have no idea), and on whether JDT's formatter has low enough dependencies to be usable by itself (again, I have no idea).

-Lex

[Ryan Ovrevik]

I know this thread has been primarily focused on configurations for ides (with the exception of Jalopy). But, we used an ant plugin/task to enforce coding style conventions in the xdoclet project.

The tool we used was Refactory and it ran as part of the build.xml. This was appealing since it didn't restrict the set of tools people used (not that this is a big deal). But must importantly it happened automatically and did not require folks to integrate settings into their ide.

rovrevik

[Miguel Méndez]

On Feb 5, 2008 10:25 AM, Lex Spoon <sp...@google.com> wrote:

On Feb 4, 2008 6:30 PM, John Tamplin <j...@google.com> wrote:

On Feb 4, 2008 4:32 PM, Lex Spoon <sp...@google.com> wrote:

Second, the formatter does go into JSNI methods.  I am posting it as is, in the hopes that some Eclipse guru could figure out how to make the formatting rules do the right thing for JSNI methods.  Alternatively, we might decide that the auto-formatted JSNI is reasonable.

I don't think what Eclipse currently does with JSNI is acceptable.

Okay.

FWIW, we will need to deal with JSNI formatting for the plugin.  It would be nice if we could have a single configuration for getting the formatting right.

 

 

  I think we can figure out the internals, override necessary parts with our own classes in the classpath ahead of the Eclipse jars, and correct the behavior there, but that will be a lot of work and fragile to Eclipse version differences.  Maybe once we have a plugin that properly formats JSNI, that plugin could be hooked into this command-line formatter tool.

If we are going to write some code, there are two other paths to consider.

One is to revive Jalopy.  Jalopy is a much smaller dependency than Eclipse, small enough that we could check the jars into the repository.  There is also an old Eclipse plugin for Jalopy, and that could also be revived if anyone was motivated enough. 

Two would be to write enough code that we only need the JDT and not all of Eclipse in order to run a code format.  We already check a lot of the JDT into the repository anyway, so it would not be a big deal to check in a slightly modified version.  The modification might use classpath overriding if that works best.


I am going back and forth about which way sounds better.  It depends on whether Jalopy is close enough where it is (maybe it sucks, I have no idea), and on whether JDT's formatter has low enough dependencies to be usable by itself (again, I have no idea).

-Lex

--
Miguel

[Martin Giese]

Hi Lex,

so you looked at jpplib, huh? I happen to be the author of that
library.

> jpplib appears to be minimal so I did not look at it much.

Yes, it's just a library that helps decide where to introduce newlines
and whitespace in any structured data, by keeping track of where
blocks start and end, and whether stuff fits on one line or not. It
could be used to write a pretty printer for Java code. It would be a
nice pretty printer in that it could do things like adding nice
linebreaks in the middle of a complicated expressions, like a lorge
condition in an if statement, for example. But somebody would have to
write that.

Yours,
Martin

[Lex Spoon]

Thank you for the followup, Martin.  It does not yet seem to be at the point where it is worth developing a new formatter, but given what you say jpplib might be a better starting point than jalopy.

The Eclipse JDT's formatter is quite close, however.  If it simply ignored the contents of JSNI comments, and simply indendented them left and right as necessary, then that would probably push it past threshold.  I went trawling for the dependencies necessary to use this formatter, and it does not look like much.  Called from the "DefaultCodeFormatter" class, you need just three jars out of Eclipse: org.eclipse.jdt.core, org.eclipse.text, and org.eclipse.equinox.common.

The text stuff is a hard dependency because the formatter computes a "TextEdit" which is a delta. The equinox dependency might not be necessary if we used CodeFomatterVisitor as the entry point rather than DefaultCodeFormatter.

I have not yet investigated whether the comment behavior is easy to override.  At least the dependencies do not seem terrible, however.  So at the moment using the JDT formatter would seem to be the leading approach.

-Lex