[Patch] Conversion options for tags GPX -> OSM

[Bjoeni]

See ticket 16550:

I would really like an option or dialog to disable the conversion of the tags in the first place. This is simply because I can upload quite a lot of my tracks with nearly no changes to OSM (e.g. some paths or tracks in the middle of nowhere), and while it isn't a big deal to remove the tags, it is unnecessary and I think that some people might miss this new feature and upload their timestamps and eles to OSM.

This patch shows a dialog asking the user whether to convert all, some or none of the tags found in the file. This can be "remembered" / saved in the settings.
The timestamps will always be kept (not the "time"-tag though!)

When some tags is remembered, a list of checked and unchecked tags is kept for future reference. In case a tag is found that hasn't been shown to the user before, the user will be asked again anyways.

When all or none is remembered, the setting will obviously be applied while converting.
Otherwise, all tags are converted and filtered later (after the dialog has been shown or when it is clear that all tags are in the positive / negative list). This is because the fields in the GPX are unknown before the conversion, but have to be shown to the user for him to decide.

The icons in the dialog are only temporary, please replace.

The settings are:

gpx.convert-tags: all, list, *ask, no
gpx.convert-tags.last: *all, list, no
gpx.convert-tags.list.yes: [list]
gpx.convert-tags.list.no: [list]

And this is my first patch / contribution to JOSM or a bigger open source project in general, so sorry if there are issues with code/style/whatever. Feedback always appreciated ;)

[Don-vip]

Thanks for the patch!
Some remarks:

• ds can be kept final as only node tags are changed, not the data set itself
• "gpx.convert-tags" is referred three times, a StringProperty could be used instead
• equals tests like convertTags.equals("list") must be inverted: "list".equals(convertTags)
• I don't remember if our parser likes tr( bot being on the same line than the string to translate, I usually keep them on the same line
• TagConversionDialogResponse: members must be declared before constructor, not after
• Fields of private inner classes can be made final

Warnings when ant clean compile pmd checkstyle javadoc is run:

    [javac] \src\org\openstreetmap\josm\gui\layer\gpx\ConvertToDataLayerAction.java:282: warning: [ClassCanBeStatic] Inner class is non-static but does not reference enclosing class
    [javac]         private class TagConversionDialogResponse {
    [javac]                 ^
    [javac]     (see http://errorprone.info/bugpattern/ClassCanBeStatic)
    [javac]   Did you mean 'private static class TagConversionDialogResponse {'?
    [javac] \src\org\openstreetmap\josm\gui\layer\gpx\ConvertToDataLayerAction.java:293: warning: [ClassCanBeStatic] Inner class is non-static but does not reference enclosing class
    [javac]         private class TagConversionDialogRadioButtonActionListener implements ActionListener {
    [javac]                 ^
    [javac]     (see http://errorprone.info/bugpattern/ClassCanBeStatic)
    [javac]   Did you mean 'private static class TagConversionDialogRadioButtonActionListener implements ActionListener {'?
...
[checkstyle] [WARN] C:\SVN\josm\core\src\org\openstreetmap\josm\gui\layer\gpx\ConvertToDataLayerAction.java:283:13: Modifier 'public' redundant. [RedundantModifier]
[checkstyle] [WARN] C:\SVN\josm\core\src\org\openstreetmap\josm\gui\layer\gpx\ConvertToDataLayerAction.java:298:13: Modifier 'public' redundant. [RedundantModifier]

Nothing to say concerning the layout, it's excellent:

It's a very good patch for a first one, congratulations! :)

[Bjoeni]

ds can be kept final as only node tags are changed, not the data set itself

Done

"gpx.convert-tags" is referred three times, a StringProperty could be used instead

Do you mean the string "gpx.convert-tags" itself or that the values are read from the settings three times? It's just read once and I changed it to a constant now. Why StringProperty?

equals tests like convertTags.equals("list") must be inverted: "list".equals(convertTags)

I liked it better the way it was, but... done.

I don't remember if our parser likes tr( bot being on the same line than the string to translate, I usually keep them on the same line

That was messed up by Eclipse. Changed.

TagConversionDialogResponse: members must be declared before constructor, not after

Done

Fields of private inner classes can be made final

Done (except TagConversionDialogResponse.sel)

[ClassCanBeStatic] Inner class is non-static but does not reference enclosing class

Done

Nothing to say concerning the layout, it's excellent
It's a very good patch for a first one, congratulations! :)

Thanks! :)

[stoecker]

See also #16579

[stoecker]

Replying to Bjoeni:

equals tests like convertTags.equals("list") must be inverted: "list".equals(convertTags)

I liked it better the way it was, but... done.

It looks better, but this way a NPE is impossible. The other way round it can throw an exception.

[Don-vip]

In 14103/josm:

fix #16572 - Conversion options for tags GPX -> OSM (patch by Bjoeni)

[Don-vip]

Ticket #16579 has been marked as a duplicate of this ticket.

[Bjoeni]

Replying to stoecker:

Replying to Bjoeni:>

equals tests like convertTags.equals("list") must be inverted: "list".equals(convertTags)

I liked it better the way it was, but... done.

It looks better, but this way a NPE is impossible. The other way round it can throw an exception.

Ok, didn't think about that. I haven't written anything in Java for quite a while and had to get used to all that weird equals stuff again...
But in general I think that rules like these usually prevent an error from being detected rather than the error itself (just like unnecessary try-catches), because actually these strings should never be null. Just my opinion though.

Replying to Don-vip:

In 14103/josm:

fix #16572 - Conversion options for tags GPX -> OSM (patch by Bjoeni)

Thank you! :)