Opened 4 years ago

Closed 3 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:

Description (last modified by zzz)

New warning in Java 8:

    [javac] i2p.i2p/core/java/src/net/i2p/data/DataHelper.java:1524: 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 bundle_messages.sh scripts must be changed to match.

Is two underscores ok? What should we use?

ref: http://mail.openjdk.java.net/pipermail/lambda-dev/2013-August/010671.html

Subtickets

#1640: options for solving #1456taskclosed
#1646: dummy sub-ticket for replyingdefectclosed
#1647: sub-ticket for adding the patch for task #1456enhancementclosed

Attachments (3)

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

Download all attachments as: .zip

Change History (15)

comment:1 Changed 4 years ago by zzz

  • Description modified (diff)

comment:2 Changed 4 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.

http://openjdk.java.net/projects/jdk9/
http://openjdk.java.net/jeps/213
https://bugs.openjdk.java.net/browse/JDK-8061549

comment:3 Changed 4 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 [​https://bugs.openjdk.java.net/browse/JDK-8065599].
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 4 years ago by kay (previous) (diff)

comment:4 Changed 4 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 4 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 4 years ago by kay (previous) (diff)

comment:6 Changed 4 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 4 years ago by kay (previous) (diff)

Changed 4 years ago by kay

removed to avoid confusion

Changed 4 years ago by kay

tidier git format-patch

comment:7 Changed 4 years ago by kay

  • Resolution set to fixed
  • Status changed from new to closed

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 4 years ago by zzz

  • Milestone changed from eventually to 0.9.23
  • Resolution fixed deleted
  • Status changed from closed to reopened

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 4 years ago by dg

  • Owner set to dg
  • Status changed from reopened to accepted

Aiming to get this in for .23.

comment:10 Changed 4 years ago by str4d

  • Keywords review-needed added

comment:11 Changed 3 years ago by dg

  • Keywords review-needed removed
  • Status changed from accepted to testing

done in 0.9.22-9 0ec09ff75939a9b5acfc5ca7dddfa6bd2eda8bd9.

comment:12 Changed 3 years ago by dg

  • Resolution set to fixed
  • Status changed from testing to closed

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

Everything else looks peachy, closing.

Note: See TracTickets for help on using tickets.