Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#8809 closed enhancement (fixed)

open Location; "no suitable download task is available"; documentation at point of error message.

Reported by: brycenesbitt Owned by: team
Priority: minor Milestone:
Component: Core Version: latest
Keywords: bug, import, xapi Cc:

Attachments (6)

Download Location_36.png (11.1 KB) - added by brycenesbitt 6 years ago.
download_task_self_documentation.diff (9.0 KB) - added by brycenesbitt 6 years ago.
Patch to document acceptable patterns
Download Location_02.png (40.6 KB) - added by brycenesbitt 6 years ago.
Post-patch version of error message
download_task_self_documentation.diff​ (9.0 KB) - added by brycenesbitt 6 years ago.
Modified version. Drop the number in front of the task. Give each task a "friendly" name. I think the task name helps group the patterns which otherwise would be overwhelming.
josm_openlocation.png (35.9 KB) - added by Don-vip 6 years ago.
Very long message
josm_openlocation_shorter.png (35.6 KB) - added by Don-vip 6 years ago.
Shorter message

Download all attachments as: .zip

Change History (32)

Changed 6 years ago by brycenesbitt

Attachment: Download Location_36.png added

comment:1 Changed 6 years ago by brycenesbitt

Description: modified (diff)

comment:2 Changed 6 years ago by brycenesbitt

(It may be the <note></note> tag in the returned XML from overpass?)

comment:3 Changed 6 years ago by Don-vip

Resolution: worksforme
Status: newclosed

Meta data are required, you need to add additional [@meta] at the end of your request to make JOSM accept it, see http://wiki.openstreetmap.org/wiki/Overpass_API/XAPI_Compatibility_Layer#Meta_Data

See accepted patterns here.

comment:4 Changed 6 years ago by brycenesbitt

Hmmm. "no suitable download task is available" was supposed to tell me that the lightly documented [@meta] qualifier was missing?

comment:5 Changed 6 years ago by Don-vip

Priority: majorminor
Resolution: worksforme
Status: closedreopened
Summary: Open Location; "no suitable download task is available"Open Location; "no suitable download task is available" (Overpass API XAPI request without [@meta])
Type: defectenhancement

I agree the message can be improved.

Changed 6 years ago by brycenesbitt

Patch to document acceptable patterns

comment:6 Changed 6 years ago by brycenesbitt

The patch makes the error message pretty geeky: but maybe that's OK in this case.
To get it to work you have to supply a URL in one of a few very specific formats.

comment:7 Changed 6 years ago by stoecker

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

Changed 6 years ago by brycenesbitt

Attachment: Download Location_02.png added

Post-patch version of error message

comment:8 in reply to:  6 Changed 6 years ago by skyper

Replying to brycenesbitt:

The patch makes the error message pretty geeky: but maybe that's OK in this case.
To get it to work you have to supply a URL in one of a few very specific formats.

A Help or More Info button leading to the wiki would be nicer and would help to keep the documentation in sync. The documentation within the code does not help normal users and the wiki is better accessible.

comment:9 Changed 6 years ago by brycenesbitt

Summary: Open Location; "no suitable download task is available" (Overpass API XAPI request without [@meta])[Patch] open Location; "no suitable download task is available"; documentation at point of error message.
Version: testedlatest

comment:10 Changed 6 years ago by brycenesbitt

Is there anything I can do, to help get this patch into the codebase?
A test case? Flowers? Thanks?
It should be reasonably shovel-ready.

comment:11 Changed 6 years ago by akks

Maybe, automate adding <ul> / <li> /etc. and leave only patterns in Download*Task and also add a "help" button?

comment:12 Changed 6 years ago by stoecker

Returning the patterns instead of whole doc and auto-generate the message looks much nicer to me also. Also the task name is not really an information for the user. A link to the wiki could help the user to understand the pattern.

Changed 6 years ago by brycenesbitt

Modified version. Drop the number in front of the task. Give each task a "friendly" name. I think the task name helps group the patterns which otherwise would be overwhelming.

comment:13 Changed 6 years ago by brycenesbitt

Out of scope of this ticket: why restrict to these URL patterns at all? How about if in a future revision any URL is loaded, and the resulting blob is passed to each download task. If the GPS task sees GPX, it can claim it. In other words switch based on content received, not URL input.

comment:14 Changed 6 years ago by akks

I had a look at the patch. Could you please introduce some "getPatterns" methods or something like this?

Pattern checking logic (url != null && url.matches(PATTERN1) ||url.matches(PATTERN2) ||...) seemes to be the same for all types,
acceptsDocumentationSummary() logic body - too. Maybe it it better to introduce base class (something like PatternBasedDownloadTask) with universal methods?

comment:15 Changed 6 years ago by brycenesbitt

My goal in this simple documentation patch is to expose the current behavior, so the user can understand what URL's are acceptable. Right now that's not possible without reading the source code.

My proposed acceptsDocumentationSummary() returns plain text in case someone writes a task that does not use a simple set of regular expressions. The author of a task thus has a free-form ability to describe what their task supports.


Comment 13 is about one possible way to completely eliminate regular expressions.
If that were done in the future, acceptsDocumentationSummary() might return something like "Compressed .osm files (bzip, bzip2, gzip or zip format)" or "GPX files with time information", instead of a regular expression.

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

comment:16 in reply to:  13 ; Changed 6 years ago by skyper

Replying to brycenesbitt:

Out of scope of this ticket: why restrict to these URL patterns at all? How about if in a future revision any URL is loaded, and the resulting blob is passed to each download task. If the GPS task sees GPX, it can claim it. In other words switch based on content received, not URL input.

Please open a new ticket for that. Thanks

comment:17 in reply to:  16 ; Changed 6 years ago by Don-vip

Replying to skyper:

Replying to brycenesbitt:

Out of scope of this ticket: why restrict to these URL patterns at all? How about if in a future revision any URL is loaded, and the resulting blob is passed to each download task. If the GPS task sees GPX, it can claim it. In other words switch based on content received, not URL input.

Please open a new ticket for that. Thanks

I don't plan to implement such a change. This is much more complicated for no real gain, the pattern approach allows us already to open a wide variety of links.
I'm ok with the pattern refactoring and the documentation patch though (they're not incompatible, I just lack of time right
now).

comment:18 in reply to:  17 ; Changed 6 years ago by skyper

Replying to Don-vip:

Replying to skyper:

Replying to brycenesbitt:

Out of scope of this ticket: why restrict to these URL patterns at all? How about if in a future revision any URL is loaded, and the resulting blob is passed to each download task. If the GPS task sees GPX, it can claim it. In other words switch based on content received, not URL input.

Please open a new ticket for that. Thanks

I don't plan to implement such a change. This is much more complicated for no real gain, the pattern approach allows us already to open a wide variety of links.
I'm ok with the pattern refactoring and the documentation patch though (they're not incompatible, I just lack of time right
now).

I see -> wontfix.

I can try to get a proper documentation in the wiki, if I find the time and power. Sorry, do not have that much time right now and I feel sometimes quite lonely as writing the wiki in two languages is often exhausting and not always fun.

Are there any stats how often the help buttons are used and/or wiki pages are browsed ?

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

comment:19 in reply to:  18 Changed 6 years ago by brycenesbitt

Replying to skyper:

I'm ok with the pattern refactoring and the documentation patch though (they're not incompatible, I just lack of time right now).

Correct: this patch is meant as a useful step in any future direction.

By documenting the patterns right on screen, and drawn from the code, it won't get out of date or be a maintenance burden like a help page or wiki.
If the next step of refactoring to a single pattern method is done, the on-screen documentation and code function will be even more tightly bound (but given the stability of this section of code, that too has limited gain).

comment:20 Changed 6 years ago by akks

In 6031/josm:

see #8809: [patch by brycenesbitt, reworked] detailed documentation for "no suitable download task is available" error (Ctrl-L)

comment:21 Changed 6 years ago by akks

I tried to do the technical job of eliminating duplicate code in the patch (no help button for now, feel free to add it), keeping the plugins compatible with existing code. Please have a look at the result and suggest future improvements.

comment:22 Changed 6 years ago by Don-vip

In 6032/josm:

see #8809 - Add Help button to "open location" error dialog + javadoc fixes in download tasks

comment:23 Changed 6 years ago by Don-vip

Summary: [Patch] open Location; "no suitable download task is available"; documentation at point of error message.open Location; "no suitable download task is available"; documentation at point of error message.

Plugins updated in [o29706], [o29708], [o29710], [o29711].

That makes a very long list that barely fits on my screen. Any idea how to reduce its size ?

Very long message

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

Changed 6 years ago by Don-vip

Attachment: josm_openlocation.png added

Very long message

comment:24 Changed 6 years ago by Don-vip

In 6033/josm:

see #8809 - Use table in "open location" error dialog to reduce its size

Result:
Shorter message

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

Changed 6 years ago by Don-vip

Shorter message

comment:25 Changed 6 years ago by Don-vip

Resolution: fixed
Status: reopenedclosed

Looks ok to my eyes now :)
Thanks all of you for ideas and patches !

comment:26 Changed 6 years ago by brycenesbitt

Thanks all!
I had no idea JOSM could import so many things!!

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.