CLDR-15381 Java formatting with prettier, tabWidth 4 #1911

btangmu · 2022-04-13T20:02:40Z

-Add package.json with custom setting for .java files: tabWidth 4

-Only java files in tools/cldr-code/src/main/java/org/unicode/cldr/util

This PR completes the ticket.

-Add package.json with custom setting for .java files: tabWidth 4 -Only java files in tools/cldr-code/src/main/java/org/unicode/cldr/util

srl295 · 2022-04-13T20:15:58Z

package.json

@@ -0,0 +1,12 @@
+{


it's… a little unusual and confusing to have a package.json at the top level. I think it might misrepresent what structure is underneath.

https://prettier.io/docs/en/configuration.html could we use a .prettierrc.json file instead?

yes, I think so

I think that might make sense.

srl295 · 2022-04-13T20:22:37Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/Annotations.java

@@ -1,5 +1,19 @@
 package org.unicode.cldr.util;

+import com.google.common.base.Joiner;


is it going to conflict with eclipse and intellij's import structure?

maybe in the same sense that it will conflict with their java formatting in general -- anytime we edit a file, regardless of which IDE or editor we use, we'll need to re-run prettier -- though we might get the IDE/editor to run it automatically, in which case i don't know if the IDE/editor will get in a fight with itself over the imports

srl295 · 2022-04-13T20:23:29Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/Annotations.java

+                "\",\"" +
+                Joiner.on("|").join(keywords) +
+                "\"},"
+            );


very much expanded

right -- compare this version of the same code in my other PR #1913:

System.out.println( "{\"" + s + "\",\"" + shortName + "\",\"" + Joiner.on("|").join(keywords) + "\"}," );

if you like that better, that's an argument for printWidth 100

i was just looking for a width parameter! Yes, let's do printwidth 100

the width is the only real issue i have otherwise lgtm

great, lets see what @macchiati and others think

macchiati · 2022-04-14T15:01:05Z

FYI: there is an eclipse format option that adds braces in if-statements that don't have them., eg the following. Those kinds of 'more than formatting' steps would be good to do periodically. if (msg.length() == 0) msg = temp; else if (temp.length() != 0) { into if (msg.length() == 0) { msg = temp; } else if (temp.length() != 0) {

…

On Wed, Apr 13, 2022 at 1:39 PM Tom Bishop ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In package.json <#1911 (comment)>: > @@ -0,0 +1,12 @@ +{ yes, I think so — Reply to this email directly, view it on GitHub <#1911 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACJLEMGVZHKHMZ6B3O6ZHZ3VE4WHDANCNFSM5TLXHZJA> . You are receiving this because your review was requested.Message ID: ***@***.***>

btangmu · 2022-04-14T15:05:19Z

I prefer always to include braces even where optional. This formatter (prettier) doesn't add or remove braces, but as you say, Mark, there's always the option to run a different formatter to add braces, and then run prettier again

macchiati · 2022-04-14T16:47:27Z

What I found strange is that prettier did make non-whitespace changes in the text, notably adding () to returns, eg return a+b+c ==> return (a+b+c) (but not always)

…

On Thu, Apr 14, 2022 at 8:05 AM Tom Bishop ***@***.***> wrote: I prefer always to include braces even where optional. This formatter (prettier) doesn't add or remove braces, but as you say, Mark, there's always the option to run a different formatter to add braces, and then run prettier again — Reply to this email directly, view it on GitHub <#1911 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACJLEMEWMVIFP2S7O2VRU23VFAX3TANCNFSM5TLXHZJA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

btangmu · 2022-04-14T17:03:47Z

What I found strange is that prettier did make non-whitespace changes in the text, notably adding () to returns, eg return a+b+c ==> return (a+b+c) (but not always)

Yes it's sort of inscrutable and sometimes does things that would never occur to me -- overall, though, I've been really happy with how it works for javascript. When the result is ugly it's sometimes because the original code is too dense, and creation of a new variable or subroutine will make it format better and be more maintainable

srl295 · 2022-04-14T17:29:04Z

What I found strange is that prettier did make non-whitespace changes in the text, notably adding () to returns, eg return a+b+c ==> return (a+b+c) (but not always)

Yes it's sort of inscrutable and sometimes does things that would never occur to me -- overall, though, I've been really happy with how it works for javascript. When the result is ugly it's sometimes because the original code is too dense, and creation of a new variable or subroutine will make it format better and be more maintainable

@btangmu is this the same prettier? It doesn't list Java nor options for java. https://prettier.io/docs/en/index.html

srl295 · 2022-04-14T17:33:49Z

OK… it's a plugin https://github.com/jhipster/prettier-java … looks like there may be some options in https://github.com/jhipster/prettier-java/blob/main/packages/prettier-plugin-java/src/options.js

btangmu · 2022-04-14T17:36:50Z

@btangmu is this the same prettier? It doesn't list Java nor options for java. https://prettier.io/docs/en/index.html

it's based on that, as a plugin:

npm install -g prettier prettier-plugin-java

https://www.npmjs.com/package/prettier-plugin-java

macchiati · 2022-05-18T22:55:02Z

The Unicodetools group is looking at this, and considering incorporating a formatter in the build process. We should coordinate with them.

macchiati · 2022-08-03T18:42:34Z

Closing, because we should at this point just import the mechanism that the Unicodetools group is using

The Unicodetools group is looking at this, and considering incorporating a formatter in the build process. We should coordinate with them.

srl295 · 2022-08-03T18:44:44Z

@macchiati maybe, would need consensus on changing all source files and invalidating many in-progress PRs. not impossible, just a bit higher impact than with unicodetools.

btangmu · 2022-08-03T19:10:37Z

I don't have any in-progress PRs myself currently -- it will be a morale-booster when we do have Java formatting!

macchiati · 2022-08-03T23:11:25Z

I agree. But also think that a "pull off the bandage" reformatting of all the source code is by far the cleanest approach. Would take some timing.

…

On Wed, Aug 3, 2022 at 11:44 AM Steven R. Loomis ***@***.***> wrote: @macchiati <https://github.com/macchiati> maybe, would need consensus on changing all source files and invalidating many in-progress PRs. not impossible, just a bit higher impact than with unicodetools. — Reply to this email directly, view it on GitHub <#1911 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACJLEMDNKVSANPPHJPRZUR3VXK42PANCNFSM5TLXHZJA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

CLDR-15381 Java formatting with prettier

99cfbb5

-Add package.json with custom setting for .java files: tabWidth 4 -Only java files in tools/cldr-code/src/main/java/org/unicode/cldr/util

btangmu requested review from macchiati and srl295 and removed request for macchiati April 13, 2022 20:14

btangmu self-assigned this Apr 13, 2022

btangmu mentioned this pull request Apr 13, 2022

CLDR-15381 Java formatting with prettier, tabWidth 4, printWidth 100 #1913

Closed

1 task

btangmu requested a review from macchiati April 13, 2022 20:16

srl295 reviewed Apr 13, 2022

View reviewed changes

macchiati closed this Aug 3, 2022

btangmu deleted the t15381_d branch September 23, 2022 14:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLDR-15381 Java formatting with prettier, tabWidth 4 #1911

CLDR-15381 Java formatting with prettier, tabWidth 4 #1911

btangmu commented Apr 13, 2022

srl295 Apr 13, 2022

srl295 Apr 13, 2022

btangmu Apr 13, 2022

srl295 Apr 13, 2022

btangmu Apr 14, 2022

srl295 Apr 13, 2022

btangmu Apr 13, 2022

srl295 Apr 13, 2022

btangmu Apr 13, 2022

srl295 Apr 13, 2022

srl295 Apr 13, 2022

btangmu Apr 13, 2022

macchiati commented Apr 14, 2022 via email

btangmu commented Apr 14, 2022

macchiati commented Apr 14, 2022 via email

btangmu commented Apr 14, 2022

srl295 commented Apr 14, 2022

srl295 commented Apr 14, 2022

btangmu commented Apr 14, 2022

macchiati commented May 18, 2022

macchiati commented Aug 3, 2022

srl295 commented Aug 3, 2022

btangmu commented Aug 3, 2022

macchiati commented Aug 3, 2022 via email

		@@ -1,5 +1,19 @@
		package org.unicode.cldr.util;

		import com.google.common.base.Joiner;

CLDR-15381 Java formatting with prettier, tabWidth 4 #1911

CLDR-15381 Java formatting with prettier, tabWidth 4 #1911

Conversation

btangmu commented Apr 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macchiati commented Apr 14, 2022 via email

btangmu commented Apr 14, 2022

macchiati commented Apr 14, 2022 via email

btangmu commented Apr 14, 2022

srl295 commented Apr 14, 2022

srl295 commented Apr 14, 2022

btangmu commented Apr 14, 2022

macchiati commented May 18, 2022

macchiati commented Aug 3, 2022

srl295 commented Aug 3, 2022

btangmu commented Aug 3, 2022

macchiati commented Aug 3, 2022 via email