GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — This adds an AOSP formatter to the Eclipse plugin. by bryanschofield · Pull Request #251 · google/google-java-format

Conversation

bryanschofield commented Feb 6, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

This is achieved by creating two different Eclipse Java Formatter implementations
that are selectable in the Java Code Style > Formatter preferences.
The google-java-format acts just the same. A new aosp-java-format
enables AOSP options.

Internally this is achieved by giving the SnippetFormatter the ability
to accept JavaFormatterOptions.

This addresses: #179

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

thc202, Stephan202, mjcctech, friederbluemle, MatthewRyanRead, kimsa, and ChristianCiach reacted with thumbs up emoji

All reactions

👍 7 reactions


          This adds an AOSP formatter to the Eclipse plugin. This is achieved

002651f

by creating two different Eclipse Java Formatter implementations
that are selectable in the _Java Code Style > Formatter_ preferences.
The `google-java-format` acts just the same. A new `aosp-java-format`
enables AOSP options.

Internally this is achieved by giving the SnippetFormatter the ability
to accept `JavaFormatterOptions`.

googlebot commented Feb 6, 2018

Copy link

Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

googlebot added the cla: no label

Feb 6, 2018

bryanschofield commented Feb 6, 2018

Copy link

Copy Markdown

Author

go go googlebot

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

googlebot commented Feb 6, 2018

Copy link

Copy Markdown

CLAs look good, thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

googlebot added cla: yes and removed cla: no labels

Feb 6, 2018

bryanschofield mentioned this pull request

Feb 6, 2018

google-java-format Eclipse plugin does not provide AOSP Style option as google-java-format IntelliJ plugin #179

Closed

Stephan202 reviewed

Feb 6, 2018

View reviewed changes

Stephan202 left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could definitely use this within our team! 👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

core/src/main/java/com/google/googlejavaformat/java/Formatter.java Outdated

		import com.google.googlejavaformat.FormattingError;
		import com.google.googlejavaformat.Newlines;
		import com.google.googlejavaformat.Op;
		import com.google.googlejavaformat.OpsBuilder;

Stephan202 Feb 6, 2018

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the imports were reordered according to Eclipse's default ordering, instead of retaining the order specified by the style guide.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bryanschofield reacted with thumbs up emoji

All reactions

👍 1 reaction

Comment thread

core/src/main/java/com/google/googlejavaformat/java/Formatter.java Outdated

		// TODO(cushon): this is only safe because the modifier ordering doesn't
		// affect whitespace,
		// and doesn't change the replacements that are output. This is not true
		// in general for

Stephan202 Feb 6, 2018

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you reformatted the code, maybe manually fix this section?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bryanschofield reacted with thumbs up emoji

All reactions

👍 1 reaction

Comment thread

eclipse_plugin/src/com/google/googlejavaformat/java/AbstractJavaFormatter.java

		* #createSnippetFormatter()} such that the indentation size corresponds with the indent level of
		* the format options used to create the snippet formatter.
		*/
		protected abstract int indentationSize();

Stephan202 Feb 6, 2018

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 maybe SnippetFormatter#indentationSize() can be made public instead? That does mean a modification to its public API, but since this caller could use it, maybe so can others.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bryanschofield Feb 6, 2018

Copy link

Copy Markdown

Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make it public, though I tend to prefer more restrictive access until a need arises. That said, I'll make change if you prefer. Please advise.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Stephan202 Feb 6, 2018

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it seems a need arose ;).

But in all seriousness: I'm not a maintainer of this project, so my advice carries little weight. Let's see what the maintainers say.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bryanschofield and koppor reacted with thumbs up emoji

All reactions

👍 2 reactions


          Removed unnecessary formatting changes caused by Eclipse.

478e67c

bryanschofield commented Feb 6, 2018

Copy link

Copy Markdown

Author

Thanks for the quick feedback! @Stephan202

Stephan202 reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aaime commented May 1, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Hi, we just switched our open source project(s) to Google AOSP and a release with this PR merged would be awesome :-)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ava1ar commented Oct 11, 2018

Copy link

Copy Markdown

Any help needed to make this merged?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tobous mentioned this pull request

Nov 14, 2018

Introduce unified code formatting for all IDEs saros-project/saros#303

Closed

Stephan202 commented Jan 27, 2019

Copy link

Copy Markdown

Contributor

The branch currently has conflicts.

@bryanschofield do you think you can resolve them?
Any feedback from the Google team on whether this change is viable would be appreciated.

(GJF is a tremendous time saver; it'd be great to see this tool adopted as widely as possible. Changes such as these definitely help achieve that.)

sormuras, kingthorin, thc202, kimsa, stefanvuckovic, ChristianCiach, rickie, and koppor reacted with thumbs up emoji

kingthorin reacted with hooray emoji

All reactions

👍 8 reactions
🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Stephan202 added a commit to PicnicSupermarket/google-java-format that referenced this pull request

Jan 27, 2019


          Make SnippetFormatter support other styles

ff4c2dd

An alternative approach would be to pass in the formatter; see
google#251.

cushon added the Eclipse label

Feb 3, 2023

brunoborges commented Jun 9, 2023

Copy link

Copy Markdown

Any update on this?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Oct 26, 2023

Switch code style to Google's Java Code Style JabRef/jabref-koppor#661

Closed

docs: clarify how this compares to google-java-format palantir/palantir-java-format#823

Open

Reviewers

1 more reviewer

Stephan202

Stephan202 left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

cla: yes Eclipse

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

bryanschofield commented Feb 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Feb 6, 2018

Uh oh!

bryanschofield commented Feb 6, 2018

Uh oh!

googlebot commented Feb 6, 2018

Uh oh!

Stephan202 left a comment

Choose a reason for hiding this comment

Uh oh!

Stephan202 Feb 6, 2018

Choose a reason for hiding this comment

Uh oh!

Stephan202 Feb 6, 2018

Choose a reason for hiding this comment

Uh oh!

Stephan202 Feb 6, 2018

Choose a reason for hiding this comment

Uh oh!

bryanschofield Feb 6, 2018

Choose a reason for hiding this comment

Uh oh!

Stephan202 Feb 6, 2018

Choose a reason for hiding this comment

Uh oh!

bryanschofield commented Feb 6, 2018

Uh oh!

aaime commented May 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ava1ar commented Oct 11, 2018

Uh oh!

Stephan202 commented Jan 27, 2019

Uh oh!

brunoborges commented Jun 9, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

bryanschofield commented Feb 6, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

aaime commented May 1, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.