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

add JAVAPKG_STYLE to supplant JAVADOC_STYLE, don’t bind by default #115

Merged
merged 5 commits into from
Feb 1, 2019
Merged

Conversation

mirabilos
Copy link
Contributor

Closes: #38

@mathieucarbou mathieucarbou added this to the 3.0 milestone Aug 16, 2016
@mathieucarbou mathieucarbou self-assigned this Aug 16, 2016
@mathieucarbou
Copy link
Owner

mathieucarbou commented Aug 16, 2016

Hi,

Would it be possible to rebase your PR and include a little update to the README.txt file (next to the JAVADOC_STYLE), plus a test against a java file with a package name ?

If a remember correctly, we have a test somewhere with every file extension type with / without header to take the input, format it and compare to the expected output.

Thanks :-)

@mirabilos
Copy link
Contributor Author

OK, will have a look at that

@@ -0,0 +1,16 @@

/*-
Copy link
Owner

@mathieucarbou mathieucarbou Aug 16, 2016

Choose a reason for hiding this comment

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

I just see the dash and your comment (#109 (comment)).

My concern is that in Java it is really not common to have this.

Why the dash ? Which IDE reformat a header based on this "hook" ?

I wonder if we shouldn't have instead 2 different header styles because /*- is not really common amongst Java devs and I am pretty sure people would like the standard one, which is just /*

/*- is not really common amongst java devs, as I explain in PR #115. If you really want this style perhaps I would suggest 2 styles: one standard without the dash (JAVAPKG_STYLE), and another one with the dash (JAVAPKG_DASH_STYLE)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Tue, 16 Aug 2016, Mathieu Carbou wrote:

/*- is not really common amongst Java devs and I am pretty sure
people would like the standard one, which is just /*

Really?

I don’t insist on it, it’s just best practice and to avoid some
tools complaning, but I’ll drop the hyphen-minus from the format.

bye,

//mirabilos

tarent solutions GmbH
Rochusstraße 2-4, D-53123 Bonn • http://www.tarent.de/
Tel: +49 228 54881-393 • Fax: +49 228 54881-235
HRB 5168 (AG Bonn) • USt-ID (VAT): DE122264941
Geschäftsführer: Dr. Stefan Barth, Kai Ebenrett, Boris Esser, Alexander Steeg

Copy link

Choose a reason for hiding this comment

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

The /*- .. */ is supposed to indicate that it shouldn't be reformatted:

http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-141999.html#350

@mirabilos
Copy link
Contributor Author

I have a rough idea wrt. the testcases, but it’s a bit complicated; will look at it later, got to go now.

@mirabilos
Copy link
Contributor Author

mirabilos commented Aug 25, 2017 via email

@mirabilos
Copy link
Contributor Author

mirabilos commented Dec 7, 2018 via email

@ctubbsii
Copy link
Contributor

ctubbsii commented Dec 7, 2018

/* will be auto-reformatted, which is not desired for licence headers, so /*- should be used for them.

Like indent's use of *INDENT-OFF*, the trailing dash (-) there seems to be very tool-specific, and not part of the Java language itself.

It seems Oracle provided a helpful example of a valid Java comment block which also matches syntax for affecting the behavior of the indent tool (although, I don't know how much I would trust poorly written docs which weren't even reviewed to the point where somebody noticed that the <blockquote> html tag was left inside the <pre> tag, thus rendering incorrectly in the browser). It would be a mistake to interpret this example as an intentional part of the Java language itself. Java has no such special tooling built-in for formatting, and thus has no special syntax for indicating to format or not.

However, other formatting tools use other conventions for restricting reformatting, and don't respect the dash. Oracle could also have provided an example of how to do it with Eclipse instead (although, Eclipse, more helpfully also has a flag to avoid formatting the "header comment"):

// @formatter:off
/*
 * Here is a block comment with some very special
 * formatting that I want indent(1) to ignore.
 *
 *    one
 *        two
 *            three
 */
// @formatter:on

@mathieucarbou mathieucarbou removed this from the 3.1 milestone Feb 1, 2019
@mathieucarbou
Copy link
Owner

I'll merge the PR.
Even if I am not convinced about the usefulness of that because it is really tool-dependant behaviour, I am willing to ease some people's life if I can.

@mathieucarbou mathieucarbou merged commit 49f3146 into mathieucarbou:master Feb 1, 2019
@mathieucarbou mathieucarbou added this to the 4.0 milestone Feb 1, 2019
@mathieucarbou mathieucarbou self-requested a review February 1, 2019 03:57
@astubbs
Copy link

astubbs commented Nov 10, 2021

How about adding an option or another style to have the copyright BEFORE the package? Docs say it should be after, but provides no reference as to why. An existing project that I'm cleaning up has it before. As I've seen in many others, and in the Oracle source files. Also see reference:
https://www.oracle.com/java/technologies/javase/codeconventions-codeexamples.html#182

I'll try with adding my own style, but seems this should be configurable?

@mathieucarbou
Copy link
Owner

How about adding an option or another style to have the copyright BEFORE the package? Docs say it should be after, but provides no reference as to why. An existing project that I'm cleaning up has it before. As I've seen in many others, and in the Oracle source files. Also see reference: https://www.oracle.com/java/technologies/javase/codeconventions-codeexamples.html#182

I'll try with adding my own style, but seems this should be configurable?

Hello,
These 2 styles should work: they place the header BEFORE the package.
Example: https://github.com/mathieucarbou/license-maven-plugin/blob/master/license-maven-plugin/src/main/java/com/mycila/maven/plugin/license/AbstractLicenseMojo.java

@astubbs
Copy link

astubbs commented Nov 10, 2021

These 2 styles should work: they place the header BEFORE the package

Thanks for responding!

Which two? JAVADOC_STYLE and JAVAPKG_STYLE? Oh that's confusing - and yes i see the linked example you provide is what I'm after.

But I wasn't getting that result by default with JAVAPKG_STYLE.

Is it due to the inline definition of the license used in the project?

I need JAVAPKG_STYLE and not JAVADOC_STYLE, but JAVAPKG_STYLE has the skip line directive:

    "^package [a-z_]+(\\.[a-z_][a-z0-9_]*)*;$",

I used this custom template and have the result I'm after:

<?xml version="1.0" encoding="UTF-8"?>
<additionalHeaders>
    <!-- Customisation to get copyright before package-->
    <!-- https://github.com/mathieucarbou/license-maven-plugin/pull/115#issuecomment-965097523-->
    <JAVAPKG_STYLE_BEFORE>
        <!-- Don't insert blank line first -->
        <firstLine><![CDATA[/*-]]></firstLine>
        <beforeEachLine><![CDATA[ * ]]></beforeEachLine>
        <endLine><![CDATA[ */EOL]]></endLine>
        <!-- don't skip any lines -->
        <!-- <skipLine><![CDATA[^package [a-z]+(\.[a-z][a-z0-9]*)*;$]]></skipLine>-->
        <firstLineDetectionPattern><![CDATA[(EOL)*(\s|\t)*/\*.*$]]></firstLineDetectionPattern>
        <lastLineDetectionPattern><![CDATA[.*\*/(\s|\t)*$]]></lastLineDetectionPattern>
        <allowBlankLines>false</allowBlankLines>
        <isMultiline>true</isMultiline>
        <padLines>false</padLines>
    </JAVAPKG_STYLE_BEFORE>
</additionalHeaders>

@mirabilos
Copy link
Contributor Author

mirabilos commented Nov 10, 2021 via email

@ctubbsii
Copy link
Contributor

@astubbs You probably just want SLASHSTAR_STYLE. That's what we use on https://github.com/apache/accumulo and it works great.

@astubbs
Copy link

astubbs commented Nov 10, 2021

No, that will cause errors or at least unnecessary warnings elsewhere. The package line has to come first.

That's odd because that's the style Oracle among many others use. Do you have any reference for that? (as per my Oracle linked example above, to show one example).

You probably just want SLASHSTAR_STYLE

Ah I see. Yeah that looks like it. The only other thing is I like the /*- style...

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

Successfully merging this pull request may close these issues.

Insert the license one line after the package
5 participants