New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 692100 link

Starred by 1 user
Status: WontFix
Owner:
thakis@chromium.org
Closed: Feb 2017
Cc:
█ dfalcant...@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

[git cl format] java: same line type annotations should be allowed

Project Member Reported by dgn@chromium.org, Feb 14 2017 
private int getTargetSheetState(float sheetHeight, float yVelocity) {
   // ...
   
   @SheetState int nextState = sStates[0];
   @SheetState int prevState = nextState;
   
   // ...
}

is "corrected" to

private int getTargetSheetState(float sheetHeight, float yVelocity) {
   // ...
   
   @SheetState
   int nextState = sStates[0];
   @SheetState
   int prevState = nextState;
   
   // ...
}

The annotation sandwich looks unappealing and not as clear as having the whole variable declaration on a single line. Because git cl now runs as part of presubmit checks, we can't even situationally use the single line notation when it looks better.

Context: https://codereview.chromium.org/2670863004/diff/80001/chrome/android/java/src/org/chromium/chrome/browser/widget/BottomSheet.java#newcode279

Nico, could you please help triage?

Comment 1 by thakis@chromium.org, Feb 14 2017

Labels: clang-format
Status: WontFix (was: Assigned)
Summary: [git cl format] java: same line type annotations should be allowed (was: [git cl format] same line type annotations should be allowed)
That sounds familiar. ...ah yeah, I implemented this a while ago since someone asked for it: http://llvm.org/viewvc/llvm-project?view=revision&revision=250422

Commit message:

"""	
clang-format/java: Break after annotations on fields in Chromium style.

Chromium follows the Android style guide for Java code, and that doesn't make
the distinction between fields and non-fields that the Google Java style guide
makes:

https://source.android.com/source/code-style.html#use-standard-java-annotations
https://google.github.io/styleguide/javaguide.html#s4.8.5-annotations

"""


I can't find the crbug where y'all asked for that, but I did that because someone complained about the old behavior that you're now requesting.

Comment 2 by dfalcant...@chromium.org, Feb 14 2017 

There's an exception for this case isn't there?  It's the "single parameterless annotation" case.

Comment 3 by thakis@chromium.org, Feb 14 2017 

I see single hits for "single parameterless annotation" on https://source.android.com/source/code-style.html#use-standard-java-annotations . I do see one on https://google.github.io/styleguide/javaguide.html#s4.8.5-annotations , but re-read that cl description: "Chromium follows the Android style guide for Java code, and that doesn't make the distinction between fields and non-fields that the Google Java style guide makes".

We use android style, not google style, for our java code.

Comment 4 by dgn@chromium.org, Feb 14 2017 

Also:

"Annotations applying to a field also appear immediately after the documentation block, but in this case, multiple annotations (possibly parameterized) may be listed on the same line; for example:

@Partial @Mock DataLoader loader;

There are no specific rules for formatting annotations on parameters, local variables, or types."

The last paragraph is the one that is most relevant for this report.

Comment 5 by thakis@chromium.org, Feb 14 2017 

That, too, is cited from the wrong guide.

Comment 6 by dgn@chromium.org, Feb 14 2017 

sorry, didn't see #c3 earlier. Makes #c4 irrelevant.

Still, the android style says: "Simple marker annotations (e.g. @Override) can be listed on the same line with the language element."
►

Sign in to add a comment