Community
Participate
Working Groups
The syntax for tagging non-externalized string literals is peculiar: Firstly, make "Non-externalized string" a warning or error: 1. Window > Preferences. 2. Turn to Java\Compiler preference page. 3. Set "Non-externalized string" to either "Warning" or "Error". 4. Click OK to close the Preferences. Now create a class as follows: public class NlsTest { public static void main(String[] args) { String platform = "Eclipse " + "2.0"; //$NON-NLS-1$ //$NON-NLS-2$ System.out.println(platform); } } - This class has TWO non-externalized string literals. - The format used above seems to be the only valid format. - The following DO NOT seem to be accepted, and perhaps should be: 1. Removal of // before second $NON-NLS-<n> entry: String platform = "Eclipse " + "2.0"; //$NON-NLS-1$ $NON-NLS-2$ 2. Insertion of whitespace between // and first $ String platform = "Eclipse " + "2.0"; // $NON-NLS-1$ //$NON-NLS-2$ 3. Usage of block-comments rather than line comments: String platform = "Eclipse " + "2.0"; /* $NON-NLS-1$ $NON-NLS-2$ */ 4. Insertion of a line break between $NON-NLS-<n> entries: String platform = "Eclipse " + "2.0"; // $NON-NLS-1$ // $NON-NLS-2$
Indeed, we probably should be more resilient, and not consider the prefix '// ' from the recognized tag.
I agree that we could accept the first three cases, but not the fourth. This tag is used to determine which strings are externalized on the same line. This is important in case of multiple line with strings on each line.
Yes, I agree with what you say about the 4th example. I included the 4th example simply as food-for-thought - typically the formatting of code has no bearing on its behavior.
This syntax cannot be accepted: /* $NON-NLS-1$ */ // $NON-NLS-2$ but I can now accept: //$NON-NLS-1$ $NON-NLS-2$ /*$NON-NLS-1$ $NON-NLS-2$ */ // $NON-NLS-1$ //$NON-NLS-2$ Is this good for you? This change won't be enough. The tools which is checking for non externalized strings will have to be updated as well.
That sounds great. How about these variations, where the $NON-NLS piece is always on the correct line, but with more complex comments surrounding: /*$NON-NLS-1$ $NON-NLS-2$ */ /* *$NON-NLS-1$ $NON-NLS-2$ */ /*Comments before *$NON-NLS-1$ $NON-NLS-2$ *Comments after */
The last two suggestions are not anymore on the correct line :-). The first one is already handled properly. This change will handle only the reporting of non- externalized strings at the compilation level, but won't change the tool that is checking for non-externalized strings. This tool will have to be updated in the same way.
Of course, my mistake. Thanks :-)
Ok, I summarize what we would like to be valid syntax: //$NON-NLS-1$ $NON-NLS-2$ // $NON-NLS-1$ //$NON-NLS-2$ /* $NON-NLS-1$ $NON-NLS-2$ */ /*$NON-NLS-1$ $NON-NLS-2$ */ The code is ready on my side, but I need to move this PR to JDT/UI to get an update the the tool. We cannot release only part of the code. Please move it back to JDT/Core when you are ready and we will synch the release of the code for the same build.
Good improvements. Adam please improve our code as well. The open question that remains is which one we insert in the future when we externalize the string the first time or when we mark the string as not to be externalized. Any suggestions on this front.
I think //$NON-NLS-1$ $NON-NLS-2$ or // $NON-NLS-1$ $NON-NLS-2$ would make most sense. Whether or not to use the leading whitespace is a matter of personal preference. BTW: I didn't realize this earlier but is the fixed code still sensitive to random whitespace between the // comment and the $NON-NLS? I think it should just strip out any whitespace so that // $NON-NLS-1$ $NON-NLS-2$ or even //$NON-NLS-1$ // $NON-NLS-2$ is valid. The relevant information is the $NON-NLS-n$ strings and everything else would ideally be ignored.
The fixed code is looking only for $NON-NLS-?. The // is not considered. This is not released for now. I won't release it as long as the tool, which is used to check non-externalized strings, is not fixed as well. If I release the change on the compiler side, you won't get an error or a warning, but the tool will still find non-externalized strings.
Adam, when you are done please forward to Martin that he can consider quick fix.
no action for 2.1
We shopuld make sime progress here for 3.0.
reopen for consideration but only a P3
for now, no plans to improve the current state
Is there any chance that this bug could be reopened and considered for Eclipse 3.4 or Eclipse 4.0?
