Modify

Opened 16 months ago

Closed 13 months ago

Last modified 13 months ago

#20467 closed enhancement (fixed)

[Patch] Show landuse type in relation menu

Reported by: mkoniecz Owned by: stoecker
Priority: normal Milestone: 21.04
Component: Core Version:
Keywords: template_report Cc: Don-vip, simon04

Description

What steps will reproduce the problem?

  1. Edit overcomplicated multipolygon such as https://www.openstreetmap.org/relation/6429589#map=15/52.0794/21.0951
  2. Enter listing of elements
  3. Become confused

What is the expected result?

landuse in listing of elements are named in way such as "landuse=forest"

What happens instead?

The have names "landuse"

Please provide any additional information below. Attach a screenshot if possible.

In this specific case forest multipolygons are incorrect as outers and should be removed. Right now it is necessary to check them one by one what is a bit confusing an obnoxious.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-01-21 23:33:21 +0100 (Thu, 21 Jan 2021)
Revision:17474
Build-Date:2021-01-22 02:30:49
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (17474 en) Linux Ubuntu 20.04.2 LTS
Memory Usage: 2540 MB / 3974 MB (1566 MB allocated, but free)
Java version: 11.0.9.1+1-Ubuntu-0ubuntu1.20.04, Ubuntu, OpenJDK 64-Bit Server VM
Look and Feel: javax.swing.plaf.metal.MetalLookAndFeel
Screen: :0.0 1920×1080 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→16×16, 32×32→32×32
Desktop environment: LXQt
Java package: openjdk-11-jre:amd64-11.0.9.1+1-0ubuntu1~20.04
Java ATK Wrapper package: libatk-wrapper-java:all-0.37.1-1
Environment variable LANG: en_US.UTF-8
libcommons-logging-java: libcommons-logging-java:all-1.2-2
fonts-noto: fonts-noto:-
Dataset consistency test: No problems found

Plugins:
+ buildings_tools (35669)
+ reverter (35688)
+ todo (30306)

Validator rules:
+ https://josm.openstreetmap.de/josmfile?page=Rules/OSMLint&zip=1
+ ${HOME}/Documents/install_moje/OSM software/manual editing and discussions/josm/resources/data/validator/deprecated.mapcss

Last errors/warnings:
- 95191.148 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 95191.942 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 95205.113 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 95214.653 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 95221.849 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 95224.405 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 95308.758 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 118857.783 W: javax.net.ssl.SSLException: Socket closed. Cause: javax.net.ssl.SSLException: Socket closed. Cause: java.net.SocketException: Socket closed
- 118857.784 W: Already here java.net.SocketException: Socket closed
- 118857.785 E: javax.net.ssl.SSLException: Socket closed. Cause: javax.net.ssl.SSLException: Socket closed. Cause: java.net.SocketException: Socket closed

Attachments (7)

screen06.png (294.4 KB) - added by mkoniecz 16 months ago.
20467.patch (1.9 KB) - added by GerdP 16 months ago.
simple patch to add new preference osm-primitives.show-full-tag
Screenshot 2021-03-26 at 21.32.42.png (282.8 KB) - added by simon04 14 months ago.
Way IDs are not helpful
Screenshot 2021-03-26 at 22.57.10.png (193.6 KB) - added by simon04 14 months ago.
The preset approach is promising
20467-presets.patch (2.8 KB) - added by simon04 14 months ago.
tr20467.patch (2.1 KB) - added by stoecker 14 months ago.
i18n.patch (513 bytes) - added by stoecker 14 months ago.

Download all attachments as: .zip

Change History (42)

Changed 16 months ago by mkoniecz

Attachment: screen06.png added

comment:1 Changed 16 months ago by GerdP

The strngs are created in a general class DefaultNameFormatter which is also used in other places like the selection list.
I always switch preference osm-primitives.showid to true to have more information and I wonder why that isn't the default, but for your case that wouldn't help much.
The code that produces the string landuse is this:

                if (n == null) {
                    n = way.hasKey("highway") ? tr("highway") :
                        way.hasKey("railway") ? tr("railway") :
                        way.hasKey("waterway") ? tr("waterway") :
                        way.hasKey("landuse") ? tr("landuse") : null;
                }

There is similar code for "building".
So, unless you use english language it wouldn't show the tag key but the i18n translation of it. I think I'd also prefer to have the full tag for all of them. I don't understand why we try to hide the actual tags or tag keys.

Changed 16 months ago by GerdP

Attachment: 20467.patch added

simple patch to add new preference osm-primitives.show-full-tag

comment:2 Changed 16 months ago by mkoniecz

simple patch to add new preference

Why preference? Maybe just change default behavior?

comment:3 Changed 16 months ago by GerdP

I have no idea how this will behave when the tag value contains unexpected strings.

comment:4 Changed 14 months ago by GerdP

Cc: Don-vip simon04 added
Summary: Show landuse type in relation menu[Patch] Show landuse type in relation menu

I've used JOSM for quite a while now with this patch and osm-primitives.show-full-tag set to true. I think it works well and want to commit it.
Should I wait for the next release?
Should the preference default be true or
Should I remove the preference osm-primitives.show-full-tag and use only the new code?

comment:5 Changed 14 months ago by stoecker

Patch looks not good to me. Have a look at getRelationTypeName(), especially the usage of trc(). That's the proper way to go to get translatable strings. No need for an option.

You also can do something like tr("{0} (building)", trc("building", value)) in case the base type must be present as well.

If possible use a translation context we already have in the presets, so translations are available (e.g. builds have values_context "building"). E.g. for landuse it's a bit more complicated. The file osm/applications/editors/josm/i18n/specialmessages.java is used to add "virtual" translations for these which aren't kept elsewhere.

comment:6 Changed 14 months ago by GerdP

I don't understand yet. The patch doesn't introduce a new i18n string. Do you want me to completely remove the old code which uses the i18n strings?

comment:7 Changed 14 months ago by stoecker

No. You add untranslated strings of the form xxx=yyy. But that's nothing which should be in the interface. User visible strings (with very few exceptions) must be translated.

comment:8 Changed 14 months ago by GerdP

I don't get it. I want to show the key=value pair, untranslated. At least I think that's what this ticket is about. I think the translated tag keys are very confusing here.

comment:9 in reply to:  8 Changed 14 months ago by stoecker

Replying to GerdP:

I don't get it. I want to show the key=value pair, untranslated. At least I think that's what this ticket is about. I think the translated tag keys are very confusing here.

We had this discussion many times over the years. JOSM UI is translated, everywhere! Outputting raw key=value at this place is simply not the right way to go. People who get "confused" simply should use the English version. It's one of the important features of JOSM that it can be used by native speakers without the need to know English.

comment:10 Changed 14 months ago by simon04

Hm, the Help/Dialog/TagsMembership outputs raw OSM tags, and copies raw OSM tags, the presets dialogs indicate which tags are affected (as tooltip), in Help/Dialog/RelationEditor the roles are raw OSM roles, ...

JOSM will never be able to abstract away the raw OSM tags, and it should not.

I agree that the output from attachment:20467.patch​ should not be used for novice users, but it easily can be added as opt-in for experts.

This patch should not stop us from striving for a naming mechanism for more types. I mean most of them are already identified via presets, so this might be a good starting point.

Changed 14 months ago by simon04

Way IDs are not helpful

comment:11 in reply to:  10 ; Changed 14 months ago by stoecker

Replying to simon04:

Hm, the Help/Dialog/TagsMembership outputs raw OSM tags, and copies raw OSM tags, the presets dialogs indicate which tags are affected (as tooltip), in Help/Dialog/RelationEditor the roles are raw OSM roles, ...

I said "With a few exceptions" which are there for a reason. You should be able to access the raw data, but it's not the default way.

JOSM will never be able to abstract away the raw OSM tags, and it should not.

I does and it should. I'm caring for I18n in JOSM since the very beginning and there has always been the idea of the English speakers that a full translation is not necessary. A lot of my opponents later agreed that a proper translation is essential for wider acceptance. JOSM usage stats clearly show the same picture. Most of the other tools followed that approach to varying degrees (because it's a lot of work to make full I18n right - I18n-mixes are much easier).

I agree that the output from attachment:20467.patch​ should not be used for novice users, but it easily can be added as opt-in for experts.

There is no need to make an expert option when it simply can be implemented properly with correct translation (see comment:5).

Last edited 14 months ago by stoecker (previous) (diff)

comment:12 Changed 14 months ago by GerdP

I think it's plain wrong to show translated tag keys or values. They are one reason why I can't work with Id. I'm a German but when I cycle on a road I might think "highway=unclassified,surface=compacted". I don't even know the exact German words for that. I think the i18n translations simply add extra work for the brain, since you have to learn the tags anyway when you want to be able to understand the OSM wiki or the warnings from the validator, like "Fehlendes Merkmal - highway=track ohne tracktype"

Anyway, a compromise could be to show translated keys together with the untranslated values (if that is what Dirk wants) and implement a preference to show the untranslated strings.

comment:13 in reply to:  12 Changed 14 months ago by stoecker

Replying to GerdP:

I think it's plain wrong to show translated tag keys or values. They are one reason why I can't work with Id. I'm a German but when I cycle on a road I might think "highway=unclassified,surface=compacted". I don't even know the exact German words for that. I think the i18n translations simply add extra work for the brain, since you have to learn the tags anyway when you want to be able to understand the OSM wiki or the warnings from the validator, like "Fehlendes Merkmal - highway=track ohne tracktype"

You are free to feel this way, and in this case I suggest simply to switch JOSM to English. All these arguments aren't new for more than a decade now.

Anyway, a compromise could be to show translated keys together with the untranslated values (if that is what Dirk wants) and implement a preference to show the untranslated strings.

No. The decision that JOSM is a fully translatable software is not open for discussion.

comment:14 Changed 14 months ago by GerdP

Sure, I use JOSM in English. I'll use my patch and maybe someone else finds a nice way to implement a fix for this ticket...

comment:15 in reply to:  11 Changed 14 months ago by simon04

Replying to stoecker:

There is no need to make an expert option when it simply can be implemented properly

Agreed!

with correct translation (see comment:5).

Maybe the strategy via preferences is easier to maintain (it avoids the need to translate the same strings again)

Changed 14 months ago by simon04

The preset approach is promising

Changed 14 months ago by simon04

Attachment: 20467-presets.patch added

comment:16 Changed 14 months ago by stoecker

What's so complicated here? Do I speak chinese? We need to add the typical street types in specialmessages.java as they aren't in the presets, but why is there such a heated discussion about something so obvious?

comment:17 Changed 14 months ago by GerdP

No heat on my side. I simply still don't understand the tricks reg. i18n. Since I don't like the idea of translated tags I get tired soon when I work on code that implements this. I am probably a very special case because I learned my first OSM tags while working for the mkgmap project, so I'll try to stay away from code that implements user interfaces ;)

comment:18 in reply to:  16 ; Changed 14 months ago by simon04

Replying to stoecker:

What's so complicated here? Do I speak chinese?

I've understood your idea of using trc. Some concerns regarding attachment:tr20467.patch

  • why restrict to highway/railway/waterway/landuse? power/amenity/barier are other popular keys according to https://taginfo.openstreetmap.org/keys, which currently often show up as ID only.
  • why manually maintain the popular tags yet again in specialmessages.java​ when our presets already contain all popular tags (thanks to org.openstreetmap.josm.data.osm.TaginfoTestIT#testCheckPopularTags)
  • you might want to use org.openstreetmap.josm.data.osm.OsmUtils#isTrue and #isFalse

comment:19 in reply to:  18 ; Changed 14 months ago by stoecker

Replying to simon04:

Replying to stoecker:

What's so complicated here? Do I speak chinese?

I've understood your idea of using trc. Some concerns regarding attachment:tr20467.patch

I used the existing code. I didn't plan to fix this issue, but provide a code example for my descriptions.

  • why manually maintain the popular tags yet again in specialmessages.java​ when our presets already contain all popular tags (thanks to org.openstreetmap.josm.data.osm.TaginfoTestIT#testCheckPopularTags)

For these presets where we don't provide value lists we don't have translations. E.g. Landuse and Streets. All these are only available in a similar, but not identical form. I wasn't able to find a solution to this topic before and also see no solution now. Reusing the mostly upper-case menu entries isn't possible. That will not work in some languages. In I18n what we as programmers would call duplication sometimes is the better solution. I'd also prefer taking the translations from a place which is not specialmessages.java which is a crude workaround.

An idea which I though about years ago: In addition to have the individual presets add e.g. a simple street or landuse preset with a value list and proper context for the types? That would have the in the proper place.

  • you might want to use org.openstreetmap.josm.data.osm.OsmUtils#isTrue and #isFalse

Good idea.

comment:20 in reply to:  19 ; Changed 14 months ago by simon04

Replying to stoecker:

An idea which I though about years ago: In addition to have the individual presets add e.g. a simple street or landuse preset with a value list and proper context for the types? That would have the in the proper place.

For presets, we already have name_template and/or match_expression. Those could be enhanced and used as building blocks for your idea:

<item name="Node Network Route relation" type="relation"
    name_template="route(?{'{network} '}!{parent() type=network(network=rcn|network=rwn|network=rhn)'?{'{ref}' | ''}'}?{'{ref} '}?{'{note} '}?{'{name}'}"
    name_template_filter="type=route (route=bicycle | route=foot | route = hiking | route = walking | route = horse)">
</item>

<item name="General preset Tower" match_expression="-tower:type=communication">
    <key key="man_made" value="tower"/>
    <multiselect key="tower:type" text="Type of tower">
        <list_entry value="bell_tower" display_value="Bell tower" icon="Tower_bell_tower.svg" />
        <list_entry value="communication" display_value="Small communication tower" icon="Tower_cantilever_communication.svg" />
        <list_entry value="cooling" display_value="Cooling" icon="Tower_cooling.svg" />
    </multiselect>
</item>

Relates to the recent change in #19012.

comment:21 in reply to:  17 Changed 14 months ago by stoecker

Replying to GerdP:

No heat on my side. I simply still don't understand the tricks reg. i18n. Since I don't like the idea of translated tags I get tired soon when I work on code that implements this. I am probably a very special case because I learned my first OSM tags while working for the mkgmap project,

You sound much like Frederik Ramm, one of the early opponents of a proper translated JOSM. He used English screenshots for a long time in his books and then switched to German interface but kept English presets and later accepted the fact that a fully translated interface is the better choice. I agree that its more complicated when you look at it from the "programmers view", but JOSM is not made for programmers. And I heard from many many JOSM users who don't speak English at all. And things like the tag-referring Validator warnings are simply ignored by them. I don' have a solution for these low-level cases, because simply adding all the tags to translation would not improve situation, but make it worse. Here I agree with you.

so I'll try to stay away from code that implements user interfaces ;)

Please not. You only need to remember that user visible texts must always be surrounded by tr(), trn(), trc(), marktr() or similar functions and that using "+" to join strings is usually forbidden. Also don't do any shorting tricks like extracting duplicate parts or similar creative things.

The functions are easy:

  • tr(): translate
  • trn(): translate when text has a text related number included
  • trc(): translate with context when the word may have different meanings in different languages (when not obvious we usually get informed by translators later and have to add contexts afterwards)
  • marktr(): mark a work as translatable which is later used in a tr() call via a variable

comment:22 in reply to:  20 Changed 14 months ago by stoecker

Replying to simon04:

Replying to stoecker:

An idea which I though about years ago: In addition to have the individual presets add e.g. a simple street or landuse preset with a value list and proper context for the types? That would have the in the proper place.

For presets, we already have name_template and/or match_expression. Those could be enhanced and used as building blocks for your idea:

I know about them. I don't believe they help here, as the problem is not the template: "{0} ({1})" is as easy as can be, but the missing (already) translated list of the required values. But that's no issue to fix here, as the code still works, only showing English texts until all the relevant values have been added (either in the presets or specialmessages.java or both).

comment:23 Changed 14 months ago by simon04

Still trying to understand what you're exactly suggesting. Something like the following combined with adopting the i18n scripts?

  • resources/data/defaultpresets.xml

    diff --git a/resources/data/defaultpresets.xml b/resources/data/defaultpresets.xml
    index 3755bd060..5a0873a35 100644
    a b  
    799799        <item name="Residential" icon="presets/transport/way/way_residential.svg" type="way,closedway" preset_name_label="true">
    800800            <link wiki="Tag:highway=residential" />
    801801            <space />
    802             <key key="highway" value="residential" />
     802            <key key="highway" value="residential" marktr="true" />
    803803            <text key="name" text="Name" />
    804804            <check key="noname" text="Street has no name" disable_off="true" />
    805805            <optional>

comment:24 in reply to:  23 Changed 14 months ago by stoecker

Replying to simon04:

Still trying to understand what you're exactly suggesting. Something like the following combined with adopting the i18n scripts?

  • resources/data/defaultpresets.xml

    diff --git a/resources/data/defaultpresets.xml b/resources/data/defaultpresets.xml
    index 3755bd060..5a0873a35 100644
    a b  
    799799        <item name="Residential" icon="presets/transport/way/way_residential.svg" type="way,closedway" preset_name_label="true">
    800800            <link wiki="Tag:highway=residential" />
    801801            <space />
    802             <key key="highway" value="residential" />
     802            <key key="highway" value="residential" marktr="true" />
    803803            <text key="name" text="Name" />
    804804            <check key="noname" text="Street has no name" disable_off="true" />
    805805            <optional>

No. Simply something like

<item name "All Streets">
  <combo key="highway" text="Highway type" values_context="highway" values="residential,service,primary,..."> 
</item>

Additional to the existing presets.

comment:25 Changed 14 months ago by simon04

From <item name="Residential" ...> we currently have trc("Highway", "Residential") (in trans_presets.java).

In addition, we'd like to get trc("highway", "residential") (possibly from <key key="highway" value="residential" />).

I don't see a point in manually maintaining values="residential,service,primary,..." ...

comment:26 Changed 14 months ago by simon04

What should generated name be in the end? "residential (highway)" or "Residential (Highway)". (For the addresses, we currently use "House ..." or "House number ...".)

comment:27 in reply to:  25 Changed 14 months ago by stoecker

Replying to simon04:

From <item name="Residential" ...> we currently have trc("Highway", "Residential") (in trans_presets.java).

Right. We can simply change the convpresets script to create the needed entries :-) It can extract keys to translate for a set of tags we want to have. Never though about this...

In addition, we'd like to get trc("highway", "residential") (possibly from <key key="highway" value="residential" />).

In English "residential (highway)".

Changed 14 months ago by stoecker

Attachment: tr20467.patch added

Changed 14 months ago by stoecker

Attachment: i18n.patch added

comment:28 Changed 14 months ago by simon04

I think we can merge the building handling into the other tags (after formatAddress):

  • src/org/openstreetmap/josm/data/osm/DefaultNameFormatter.java

    diff --git a/src/org/openstreetmap/josm/data/osm/DefaultNameFormatter.java b/src/org/openstreetmap/josm/data/osm/DefaultNameFormatter.java
    index 84640adca..a6e895a63 100644
    a b public class DefaultNameFormatter implements NameFormatter, HistoryNameFormatter 
    230230                if (n == null) {
    231231                    n = way.get("ref");
    232232                }
    233                 if (n == null) {
    234                     n = way.hasKey("highway") ? tr("highway") :
    235                         way.hasKey("railway") ? tr("railway") :
    236                         way.hasKey("waterway") ? tr("waterway") :
    237                         way.hasKey("landuse") ? tr("landuse") : null;
    238                 }
    239233                if (n == null) {
    240234                    n = formatAddress(way);
    241235                }
    242                 if (n == null && way.hasKey("building")) {
    243                     n = tr("building");
     236                if (n == null) {
     237                    for (String key : Arrays.asList(marktr("highway"), marktr("railway"), marktr("waterway"), marktr("landuse"), marktr("building"))) {
     238                        if (way.hasKey(key) && !way.isKeyFalse(key)) {
     239                            /* I18N: first is highway, railway, waterway, landuse or building type, second is the type itself */
     240                            n = way.isKeyTrue(key) ? tr(key) : tr("{0} ({1})", trcLazy(key, way.get(key)), tr(key));
     241                            break;
     242                        }
     243                    }
    244244                }
    245245                if (n == null || n.isEmpty()) {
    246246                    n = String.valueOf(way.getId());

comment:29 in reply to:  28 Changed 14 months ago by stoecker

Replying to simon04:

I think we can merge the building handling into the other tags (after formatAddress):

Though about it as well. Can't remember any reason why it is this way round. Probably simply "historic" :-)

comment:30 Changed 14 months ago by stoecker

Milestone: 21.04

comment:31 Changed 14 months ago by simon04

Owner: changed from team to stoecker

comment:32 Changed 13 months ago by stoecker

Resolution: fixed
Status: newclosed

In 17810/josm:

fix #20467 - support more relation types details

comment:33 Changed 13 months ago by stoecker

In 35737/osm:

see #20467 - support more relation types details

comment:34 Changed 13 months ago by GerdP

In 17812/josm:

see #20467: Show landuse type in relation menu

  • add missing import for marktr

comment:35 Changed 13 months ago by simon04

In 17825/josm:

see #20467 - Checkstyle

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain stoecker.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.