Single line comments on "case:" lines forced to next line

[anaran]

As tested at minutes ago at http://jsbeautifier.org/
following code

switch (a) {
case "ok"://$NON-NLS-0$
    break;
case "ko"://$NON-NLS-0$
    break;
}

gets beautified as

switch (a) {
case "ok":
    //$NON-NLS-0$
    break;
case "ko":
    //$NON-NLS-0$
    break;
}

which breaks the purpose of these i18n annotations.

I first noticed the problem in Orion plugin
http://mamacdon.github.io/0.5/plugins/beautify/jsbeautify.html
as well.

[evocateur]

Can you give us a reference for this format? Why would dropping it to the next line be bad? Is there a consistent, easily identifiable pattern for these comments?

[anaran]

Hi Daniel,

see
http://wiki.eclipse.org/Orion/Internationalization
for background information and the format itself.
It is widely used for i18n in Java and JavaScript.

Moving the comment markup to the next line will defeat its purpose:

It will flag the string again as
"Non externalized string literal"

I have not found a formal spec, but EOL comments matching /\bNON-NLS\b/ should be left alone.

Ah, here is the source:
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/tree/bundles/org.eclipse.orion.client.core/web/plugins/nonnlsPlugin.html?id=3b3d879b8dd8957f41f114682e8642bf8d9f7b47

Best regards!
Adrian

See also
http://dev.eclipse.org/mhonarc/lists/orion-dev/msg01710.html
http://stackoverflow.com/questions/3593771/how-to-place-non-nls-1-kind-of-comments-in-eclipse

[bitwiseman]

You have this:

switch (a) {
case "ok"://$NON-NLS-0$
    break;
case "ko"://$NON-NLS-0$
    break;
}

It looks like if it were beautified as follows it would it still be acceptable:

switch (a) {
case "ok": //$NON-NLS-0$
    break;
case "ko": //$NON-NLS-0$
    break;
}

Is that correct?

I could see this as a potentially valid case for any // comment. Similar to a number of other tokens we could precede them with a space unless preserver_newline is set, in which case we keep the newline if it was present in the input.

[anaran]

@bitwiseman You are correct.
I should have had the space before // in my example.

Regarding preserver_newline;
//$NON-NLS-0$
on a line by itself would be useless in all cases.

I am going to mention a different formatting issue here just because it may be connected:
I think // comments starting in the first column should not be indented.
Has this issue come up before?

Let me know if I should elaborate or raise a separate issue for this.

[bitwiseman]

Non-indent first column // comments: Yes, file a separate issue.

Regarding preserve_newline and //$NON-NLS-0$: Yes, I understand that it would have no meaning. My point was that we could make the default behavior agree with this requirement, while also maintaining support for the current behavior.

And actually now that I'm running the scenario in my head, I see this really not a big change at all and preserve_newline has nothing to do with it:

/* Input */
if (a) { // This is a plain old comment
    b();
    // This is also a plain old comment
    c = "TOKEN_NAME"; //$NON-NLS-0$
    switch (c) {
    case "TOKEN_NAME": //$NON-NLS-0$
        break;
    default:
        break;
    }
}

/* Old Output */
if (a) { // This is a plain old comment
    b();
    // This is also a plain old comment
    c = "TOKEN_NAME"; //$NON-NLS-0$
    switch (c) {
    case "TOKEN_NAME": 
        //$NON-NLS-0$
        break;
    default:
        break;
    }
}


/* New Output */
if (a) { // This is a plain old comment
    b();
    // This is also a plain old comment
    c = "TOKEN_NAME"; //$NON-NLS-0$
    switch (c) {
    case "TOKEN_NAME": //$NON-NLS-0$
        break;
    default:
        break;
    }
}

Okay, I can do this.

[anaran]

Great, thanks!

[bitwiseman]

Updated title and label - general bug, not specific to NON-NLS.