Modify

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#9327 closed enhancement (fixed)

[beta Patch] De-duplicate contents of defaultpresets

Reported by: simon04 Owned by: team
Priority: major Milestone: 14.01
Component: Internal preset Version:
Keywords: deduplication, reference Cc: Pedja, kendzi

Description

To reduce the length and duplicated parts of defaultpresets.xml, I tested the following idea: For <optional> elements, an id="x" may be specified. When inserting a <reference ref="x"/> element any time later, the corresponding optional elements are retrieved again.

Please see the attached patch for details and an example on the barrier presets. What do you think?

Attachments (4)

9327v1.patch (13.8 KB) - added by simon04 6 years ago.
9327v2.patch (27.8 KB) - added by simon04 6 years ago.
9327v3.patch (249.8 KB) - added by simon04 5 years ago.
chunk_problem.patch (8.1 KB) - added by Don-vip 5 years ago.
Syntax accepted but presets do not work as expected

Download all attachments as: .zip

Change History (33)

Changed 6 years ago by simon04

Attachment: 9327v1.patch added

comment:1 Changed 6 years ago by Don-vip

Priority: normalmajor

A big +1 on the idea :)
For implementation, consider also the ideas proposed here:

  • #7556: Introducing chuks of settings in Presets
  • #8958: Adding „library” section to presets. (idea)
  • #9066: presets: including other preset extracts

We definitively need this to maintain presets in the long term.

Last edited 6 years ago by Don-vip (previous) (diff)

Changed 6 years ago by simon04

Attachment: 9327v2.patch added

comment:2 Changed 6 years ago by simon04

Summary: [alpha Patch] De-duplicate contents of defaultpresets[beta Patch] De-duplicate contents of defaultpresets

v2: A more concise and flexible implementation using <chunk id="x">...</chunk> at the beginning of the preset file, which can be used using <reference ref="x"/>.

After reading through the mentioned tickets, I think there're similar ideas with small differences in the implementation (separate files vs. designated parts in main preset file, etc.). I somewhat prefer a single file (as in the current path) since it doesn't require a change in the i18n process, and it permits a simple find operation of the editor within the same file when editing presets.

comment:3 Changed 6 years ago by A_Pirard

I'm the reporter of #9066.
My idea is that someone who wants to make a specialized set of the existing presets he uses most for a special activity should not copy the definitions he clones but link to them. The link is general, like an URL, and may fetch from any source. The source needs not to be modified. It is not only a question of internal optimization of the default presets, it's using them, or any, from the outside.
The intention was to reuse whole presets as is, but after being erroneously duplicated with #8958, I imagined that the two ideas could be merged, i.e. link with modification. Modification, of course, can fail if an out of control external source gets modified incompatibly.
Any method to do that is much welcome. Thanks!!!

comment:4 in reply to:  3 Changed 6 years ago by simon04

Replying to A_Pirard:
I'm not sure to fully grasp your idea. Please give a small example to illustrate.

comment:5 Changed 6 years ago by A_Pirard

Hi. Much explanation in the other tickets. Simple. Suppose you survey streets for example.
Instead of repeatedly fumbling for the features scattered in a number of 3-level trees, it speeds up work to make a custom arrangement of the presets you use most. And a link is obviously always more interesting than a copy in case the preset is updated. There is probably much duplication of outdated presets in the private and public preset mass.
Being able to override the contents, especially with a chunk you link to, is a real plus. For example, every time I read about tagging the street address of something, I recall to not forget the website (which contains the address). Look at supermarket aso, no website. Your idea and mine will make possible the inclusion of a "coordinates" chunk for address, phone,... not forgetting website.

comment:6 Changed 6 years ago by stoecker

See also #1877 suggestion c.

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

comment:7 Changed 6 years ago by A_Pirard

I happened to come across http://josm.openstreetmap.de/wiki/Presets/BENELUX and what he says in Description is an example of what you ask for.

comment:8 Changed 6 years ago by simon04

Replying to A_Pirard:
How exactly would your idea be written to the defaultpresets.xml file? What cannot be done with the syntax from attachment:9327v2.patch?

comment:9 Changed 5 years ago by simon04

v3: Manually extracted frequent combinations. Here's the change in terms of wc (lines, words, bytes):

old   7545  28113 525237 data/defaultpresets.xml
new   7039  21667 447575 data/defaultpresets.xml

Changed 5 years ago by simon04

Attachment: 9327v3.patch added

comment:10 Changed 5 years ago by A_Pirard

My #9066 idea is not to write anything to defaultpresets.xml but to private preset files.
Have you read #9066 and the mentioned preset file?
Please show an example of how it can be done with your syntax.

comment:11 in reply to:  10 ; Changed 5 years ago by simon04

Replying to A_Pirard:

My #9066 idea is not to write anything to defaultpresets.xml but to private preset files.
Have you read #9066 and the mentioned preset file?
Please show an example of how it can be done with your syntax.

I read it several times, but didn't get the idea. That's why I asked for an example in comment:4 and comment:8.

The title of this ticket is "De-duplicate contents of defaultpresets" and that's why the patch de-duplicates the defaultpresets.

comment:12 in reply to:  11 Changed 5 years ago by A_Pirard

The initial idea of #9066 is to include without duplicating it any preset element from any file to another preset file. It was erroneously duplicated to your more restricted "de-duplicating [internal] contents of defaultpresets" concern. After pointing out the difference, I wrote that the same patch could achieve both goals at the same time.
My URL-like concept can identify both the XML element and the file in which it is comes from.
I just can't figure how to explain that something LIKE
<include name=filename(=URL)/group-name/[item-name]>
can do what you want by including almost anywhere almost any XML element picked from any file
while
<reference ref="highway_base" />
can obviously not pick an XML element from any file
and hence not do what I want (as you say it could).

For example:
<include name=/http://josm.openstreetmap.de/browser/trunk/data/defaultpresets.xml/Highways/Streets/Motorway>
or
<include name=/http://josm.openstreetmap.de/browser/trunk/data/defaultpresets.xml/group.name=Highways/group.name=Streets/item.name=Motorway>
or
<include name="chunk.id=barrier">
or WHATEVER EQUIVALENT SYNTAX YOU DEVISE, it's up to you if you do that.

comment:13 Changed 5 years ago by Don-vip

Please no need for uppercase speech, it's rude.

I think I understand your request now, we could simply use XML namespaces for that.

ref=bar => item defined in current preset file
ref=foo:bar => item defined in preset file referenced by namespace foo

As why I referenced your ticket in this one, it's because it's a similar (but not identical) topic we must have in mind while implementing this ticket, that's all.

comment:14 in reply to:  13 Changed 5 years ago by A_Pirard

Replying to Don-vip:

Please no need for uppercase speech, it's rude.

Were computers invented by rude people? :-) (there was no lowercase before)
No, it's just emphasis. No worse than JOSM ;-)

I think I understand your request now, we could simply use XML namespaces for that.

ref=bar => item defined in current preset file
ref=foo:bar => item defined in preset file referenced by namespace foo

If any file can be assigned a namespace, that's fine.
But remember that any XML element (at least group and item) can be referenced, not just made up ones like chunk.

Thanks for your attention.


comment:15 Changed 5 years ago by Don-vip

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

comment:16 Changed 5 years ago by Don-vip

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

comment:17 Changed 5 years ago by Don-vip

Cc: Pedja kendzi added; deduplication reference removed
Keywords: deduplication reference added
Milestone: 14.01

The patch looks good, let's give it a try !
We can then close this ticket and try to implement external references with namespaces in #9066.

comment:18 Changed 5 years ago by simon04

Resolution: fixed
Status: newclosed

In 6558/josm:

fix #9327 - De-duplicate contents of defaultpresets.xml by introducing <chunk id="X"> and <reference ref="X" />

comment:19 Changed 5 years ago by Don-vip

In 6559/josm:

see #9327 - typo + minor refactoring error

Changed 5 years ago by Don-vip

Attachment: chunk_problem.patch added

Syntax accepted but presets do not work as expected

comment:20 Changed 5 years ago by Don-vip

Resolution: fixed
Status: closedreopened

I tried to refactor railways but it does not work, I think it's not currently possible to mix references, combos, then references ?

Btw I'm leaving tomorrow morning for new year's eve vacation, I'll see progress updates, if any, next year :)

Take a break Simon as well, you have earned it ;)

Happy new year !

comment:21 Changed 5 years ago by simon04

In principle this combination should work. I'll investigate.

Vincent, enjoy your vacation and Happy New Year! See you … :-)

comment:22 Changed 5 years ago by simon04

Resolution: fixed
Status: reopenedclosed

In 6562/josm:

fix #9327 - tagging presets: fix handling of nested <reference>s, refactor railway presets

comment:23 Changed 5 years ago by A_Pirard

Replying to simon04:

A possible implementation [for #9066] (already containing a patch) is being discussed in #9327.

I just wrote #9066 comment 6 to restate even more precisely, now in terms of more precise syntax, how a slightly wider scope of this patch would satisfy not only the de-duplication of defaultpresets.xml by its implementors but also the de-duplication of anypresets.xml by its author who duplicates for convenience items of other files.
Note that the latter does not have access to add chunks and ids to the other files.

comment:24 Changed 5 years ago by aceman

Great consolidation, thanks! This will surely prevent some current or future divergence of similar preset blocks.
Are these blocks intentionally not referencing highway_yesno_incline_oneway_lit and opencoding the elements instead? Is it because you really want to insert motorroad/oneway:bicycle into the middle of the block?

397 <reference ref="highway_base" />
398 <check key="oneway" text="Oneway" />
399 <check key="motorroad" text="Motorroad" />
400 <reference ref="highway_yesno_incline" />
401 <check key="lit" text="Lit" />

415 <reference ref="highway_base" />
416 <check key="oneway" text="Oneway" />
417 <check key="motorroad" text="Motorroad" />
418 <reference ref="highway_yesno_incline" />
419 <check key="lit" text="Lit" />

552 <reference ref="surface" />
553 <check key="oneway" text="Oneway" />
554 <check key="oneway:bicycle" text="Oneway for bicycle" />
555 <reference ref="highway_yesno_incline" />
556 <check key="lit" text="Lit" />

comment:25 Changed 5 years ago by A_Pirard

#7797 comment:7 speaks of you.

comment:26 Changed 5 years ago by simon04

In 6570/josm:

see #9327 - tagging presets: harmonize some presets

comment:27 in reply to:  24 Changed 5 years ago by simon04

Replying to aceman:

Are these blocks intentionally not referencing highway_yesno_incline_oneway_lit and opencoding the elements instead? Is it because you really want to insert motorroad/oneway:bicycle into the middle of the block?

Those haven't been found in the manual search+replace process. I changed the first two of them, but I'd stick with the current "Bicycle Road" since otherwise the order of "Oneway" bla bla "Oneway for bicycle" is really strange.

comment:28 Changed 5 years ago by Don-vip

In 6701/josm:

see #9327 - refactor parking presets

comment:29 Changed 5 years ago by Don-vip

In 6704/josm:

see #9327 - refactor name/wikipedia presets items + places

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
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.