Opened 6 years ago

Closed 6 years ago

#1456 closed defect (fixed)

rename _() methods

Reported by: zzz Owned by: dg
Priority: maintenance Milestone: 0.9.23
Component: api/general Version: 0.9.17
Keywords: easy gruntwork Cc:
Parent Tickets: Sensitive: no

Description (last modified by zzz)

New warning in Java 8:

    [javac] i2p.i2p/core/java/src/net/i2p/data/ warning: '_' used as an identifier
     [javac]             return _("n/a");
     [javac]                    ^
     [javac]   (use of '_' as an identifier might not be supported in releases after Java SE 8)

Java 9 won't be here anytime soon but the warning is annoying.

All scripts must be changed to match.

Is two underscores ok? What should we use?



#1640: options for solving #1456closed
#1646: dummy sub-ticket for replyingclosed
#1647: sub-ticket for adding the patch for task #1456closed

Attachments (3)

task-1456-diff.patch (9 bytes) - added by kay 6 years ago.
removed to avoid confusion
0001-Maintenance-changes-from-_-to-_t-in-order-to-prepare.patch (810.7 KB) - added by kay 6 years ago.
tidier git format-patch
0002-Apply-changes-for-task-1456-also-to-apps-i2ptunnel-j.patch (17.5 KB) - added by kay 6 years ago.
missed one jsp

Download all attachments as: .zip

Change History (15)

comment:1 Changed 6 years ago by zzz

Description: modified (diff)

comment:2 Changed 6 years ago by zzz

Confirmed to be illegal in Java 9. Will need to fix this by early 2016, or whenever beta builds of Java 9 become available.

comment:3 Changed 6 years ago by kay

Options for solving the _( issue:

The double underscore solution might be harder too read.
So, maybe try something like just one letter, e.g. t()
or _t() for 'translate'?
The latter would sort of correspond to _x() naming.

In addition, the underscore might eventually be put to special use:

 The use of the variable name _ in any context is discouraged.
 Future versions of the Java programming language may reserve this
 name as a keyword and/or give it special semantics.

See [​].
If wanting to be very careful and avoid future changes, maybe another reason not to choose double underscore.

Thoughts on __(, _t(, or t(?

PS: Once a decision is reached I would volunteer to make the changes.

Some stats for the Java Code w/o shell scripts:

The underscore-brace combination _( shows up
1735 times (993 times preceded by another brace (_(
and 742 times preceded by whitespace.

Last edited 6 years ago by kay (previous) (diff)

comment:4 Changed 6 years ago by zzz

_t isn't bad. The actual new name doesn't matter so much. Just have to decide and then do the work. Still not a rush.

comment:5 Changed 6 years ago by kay

I agree _t() looks best.

  • So I would apply these changes and upload a patch somewhere. Where?
  • Which branch should I base the patch on?
Last edited 6 years ago by kay (previous) (diff)

comment:6 Changed 6 years ago by kay

Attached a git-format-patch.

ant fulltest ran successfully after applying the patch.

But I had to comment out the non-existing router scala tests.

Last edited 6 years ago by kay (previous) (diff)

Changed 6 years ago by kay

Attachment: task-1456-diff.patch added

removed to avoid confusion

Changed 6 years ago by kay

tidier git format-patch

comment:7 Changed 6 years ago by kay

Resolution: fixed
Status: newclosed

Using both git-format-patches the test suite passes successfully (as above)
and buildclean finishes successfully too.

I close this ticket as fixed.
Please, review and verify, and re-open, if necessary.

comment:8 Changed 6 years ago by zzz

Milestone: eventually0.9.23
Resolution: fixed
Status: closedreopened

Please don't close the ticket as fixed until it's reviewed by somebody and applied to trunk in our source control. Otherwise we'll forget about it. That's what trac is for. Perhaps we can get it into 0.9.23.

comment:9 Changed 6 years ago by dg

Owner: set to dg
Status: reopenedaccepted

Aiming to get this in for .23.

comment:10 Changed 6 years ago by str4d

Keywords: review-needed added

comment:11 Changed 6 years ago by dg

Keywords: review-needed removed
Status: acceptedtesting

comment:12 Changed 6 years ago by dg

Resolution: fixed
Status: testingclosed

News was fixed in an immediate follow-up commit (d81341bcf620ced38e1eb733b240dfa9ecd2bb6e).

Everything else looks peachy, closing.

Note: See TracTickets for help on using tickets.