[PATCH] Include changeset in note comment if feasible

[qeef]

When closing a note, check for the changeset in the changeset cache. If
there is a changeset with the comment that contains a link to the note
being closed, put the link to the changeset as the default text of the
closing note comment.

[qeef]

This change tends to improve the navigation between related changeset and note, originally raised here: ​https://lists.sr.ht/~qeef/damn-project/%3Ccce27ab08172b1a61983902570dd15c1%40gusl.org%3E

[taylor.smock]

Feedback on the patch:

• We typically use camelCase for variables instead of underscores. So note_url_short -> noteUrlShort.
• I think the note_url_short and note_url_long variables can be in the if statement scope
• We should probably be looking for "osm.org/note/" and "openstreetmap.org/note/" instead of the full URL. In addition, we should probably be looking for the current OSM API endpoint, maybe special casing it for the short osm.org (use the OsmApi class and JosmUrls.getDefaultOsmApiUrl method)
• Add @since xxx to the new showNoteDialog method javadocs

[qeef]

Thank you for the review and sorry for the recurring issues with snake_case variables.

In addition, we should probably be looking for the current OSM API endpoint, maybe special casing it for the short osm.org (use the OsmApi class and JosmUrls.getDefaultOsmApiUrl method)

I am sorry, but I am missing why. This patch downloads nothing from OSM, it just checks just uploaded changesets for a link to the note being closed (if I understand ChangesetCache correctly).

[taylor.smock]

JOSM isn't used solely on OSM.org.

For example, if someone fixes a note on OpenHistoricalMap, this code should still work if they reference the note in their changeset comment.

Example:
A changeset with the comment "I fixed ​https://openhistoricalmap.org/note/1" should autofill the closing comment with the changeset id.

[qeef]

JOSM isn't used solely on OSM.org.

I was thinking about that. Please, check the v3. Thank you!

[taylor.smock]

No worries -- it is something that we tend to try and think about. The one exception is validations, presets, and paintstyles, which are all configurable.

Anyway, why not

- if (cs.getComment().indexOf(noteUrlShort) > -1 || cs.getComment().indexOf(noteUrlLong) > -1) {
+ if ((Config.getUrls().getDefaultOsmApiUrl().equals(OsmApi.getOsmApi().getServerUrl()) && cs.getComment().indexOf(noteUrlShort) > -1) || cs.getComment().indexOf(noteUrlLong) > -1) {

If you've got a reason, go for it.

[qeef]

I am sorry, it looks like I missed your point. Patch v4 applies default comment for the note to be closed only for the default OSM instance, i.e., osm.org or openstreetmap.org.

[taylor.smock]

LGTM. I'll pull it down and test it (and hopefully write some unit tests for it, but UI stuff can be a bit of a PITA to write tests for).

Side node: do you want me to use qeef when attributing your patch (e.g. Fix #23179: Include changeset in note comment if feasible (patch by qeef))?

[qeef]

Thank you! No need for the attribution.

[qeef]

I have yet uploaded follow-up patch v2-0002-... with the generalization to other instances if you think it's good idea. Thank you.

[taylor.smock]

In 18839/josm:

Fix #23179: Include changeset in note comment if feasible (patch by qeef, modified)

Modifications are as follows:

• Unit tests
• Better note matching
• Find multiple changesets referring the same note

[taylor.smock]

Ticket retargeted after milestone deleted

[taylor.smock]

Ticket retargeted after milestone deleted