Modify

Opened 3 years ago

Closed 3 years ago

#20310 closed enhancement (fixed)

[Patch] ImageImporter should understand web URL's

Reported by: taylor.smock Owned by: team
Priority: normal Milestone: 21.02
Component: Core remotecontrol Version:
Keywords: Cc:

Description

Use cases:

  • Image files stored on the web (this will only allow single files to be added at a time)

For example, at this time, StreetComplete creates links in notes to pictures. (To download planet notes, see https://planet.openstreetmap.org/notes/ ).

This can be used to fairly quickly resolve a note in MapRoulette via remote control.

I.e., a parsing program could take the note db, and for every note that has a url that looks like https?://.*\.jpg, create a remote control URL that looks like http://127.0.0.1:8111/open_file?filename=<url>.

Attachments (4)

20310.patch (2.4 KB ) - added by taylor.smock 3 years ago.
Allow JpgImporter to import remote images
20310.1.patch (15.3 KB ) - added by taylor.smock 3 years ago.
Now with options
20310.2.diff (8.5 KB ) - added by Bjoeni 3 years ago.
20310.deprecated_update.patch (3.1 KB ) - added by taylor.smock 3 years ago.
Update deprecated methods used elsewhere in JOSM core

Download all attachments as: .zip

Change History (23)

by taylor.smock, 3 years ago

Attachment: 20310.patch added

Allow JpgImporter to import remote images

comment:1 by skyper, 3 years ago

Summary: JpgImporter should understand web URL's[Patch] JpgImporter should understand web URL's

Is this worth an own settings option to en-/disable the feature?

comment:2 by taylor.smock, 3 years ago

I didn't think about that, but some people would probably appreciate that.

Locations:

  • Advanced Preferences (no UI element, good initial first step)
  • Remote Control (this is probably the best place for the UI element, since a user would otherwise expect it to "just work")
    • If this is the case, then passing Strings around might be better than passing a File around, or having some method to get a "real" pathname (new File(String) replaces // with /).
  • Some other location

comment:3 by skyper, 3 years ago

As you need to enable remote control in preferences, I think it could go with all the other options there and its default should be enabled like all other options.

by taylor.smock, 3 years ago

Attachment: 20310.1.patch added

Now with options

comment:4 by taylor.smock, 3 years ago

Notes on attachment:20310.1.patch:

  • Replace boolean option with enum option (specifically, recordHistory)
  • Mark several methods as deprecated (docs have @deprecated .*[Ss]ince xxx.* -- hopefully, the @since xxx replacement script also replaces those)
  • When storing enums, use EnumSet instead of an Array. Most methods take an array instead of a set.

comment:5 by taylor.smock, 3 years ago

2 week keep-alive ping.

comment:6 by taylor.smock, 3 years ago

Keep alive ping (am I missing something?)

comment:7 by taylor.smock, 3 years ago

2 week keep alive ping.

comment:8 by stoecker, 3 years ago

Milestone: 21.02

comment:9 by Don-vip, 3 years ago

Resolution: fixed
Status: newclosed

In 17534/josm:

fix #20310 - Allow JpgImporter to import remote images (patch by taylor.smock)

comment:10 by Don-vip, 3 years ago

Resolution: fixed
Status: closedreopened

@Taylor the newly deprecated API is still being used in JOSM core, can you please provide a patch?

compile:
    [javac] Compiling 1640 source files to /tmp/josm/josm/build
    [javac] /tmp/josm/josm/src/org/openstreetmap/josm/gui/MainApplication.java:1278: warning: [deprecation] openFiles(List<File>,boolean) in OpenFileAction has been deprecated
    [javac]             tasks.add(OpenFileAction.openFiles(fileList, true));
    [javac]                                     ^
    [javac] /tmp/josm/josm/src/org/openstreetmap/josm/gui/io/RecentlyOpenedFilesMenu.java:87: warning: [deprecation] setRecordHistory(boolean) in OpenFileTask has been deprecated
    [javac]             task.setRecordHistory(true);
    [javac]                 ^
    [javac] /tmp/josm/josm/src/org/openstreetmap/josm/gui/datatransfer/importers/FilePaster.java:36: warning: [deprecation] setRecordHistory(boolean) in OpenFileTask has been deprecated
    [javac]         task.setRecordHistory(true);
    [javac]             ^
    [javac] 3 warnings

comment:11 by Bjoeni, 3 years ago

Vincent, is it possible that my patch in #20341 / [17548] conflicted with this patch?

Because the patch from the other ticket was uploaded before this one was committed, and it seems the newly created ImageImporter does not reflect the changes that were made to JpgImporter in this ticket anymore.

Last edited 3 years ago by Bjoeni (previous) (diff)

in reply to:  11 comment:12 by Don-vip, 3 years ago

Replying to Bjoeni:

Vincent, is it possible that my patch in #20341 / [17548] conflicted with this patch?

Totally possible indeed.

comment:13 by Bjoeni, 3 years ago

Summary: [Patch] JpgImporter should understand web URL's[Patch] ImageImporter should understand web URL's

I merged the changes to ImageImporter, but didn't test anything.

@Taylor could you please confirm if it works as intended?

by Bjoeni, 3 years ago

Attachment: 20310.2.diff added

in reply to:  13 comment:14 by taylor.smock, 3 years ago

Replying to Bjoeni:

I merged the changes to ImageImporter, but didn't test anything.

@Taylor could you please confirm if it works as intended?

It does work as intended.

curl -O 'http://127.0.0.1:8111/open_file?filename=https://mongoose-d3mo.s3-us-west-2.amazonaws.com/Mexico/Guadalajara/ipad_3/day_2/front/BlackVue/Record/20200612_201036_NF/20200612_201036_NF_000111.jpg' is what I used to test (note: that URL might stop working at some point in time).

by taylor.smock, 3 years ago

Update deprecated methods used elsewhere in JOSM core

in reply to:  10 comment:15 by taylor.smock, 3 years ago

Replying to Don-vip:

@Taylor the newly deprecated API is still being used in JOSM core, can you please provide a patch?

Done. I should have done this earlier.

comment:16 by Don-vip, 3 years ago

Resolution: fixed
Status: reopenedclosed

In 17556/josm:

fix #20310 - Update deprecated methods used elsewhere in JOSM core (patch by taylor.smock, modified)

comment:17 by Bjoeni, 3 years ago

@Vincent attachment:20310.2.diff is not applied yet - not sure if you have seen it :)

comment:18 by Don-vip, 3 years ago

Resolution: fixed
Status: closedreopened

ah, sorry

comment:19 by Don-vip, 3 years ago

Resolution: fixed
Status: reopenedclosed

In 17558/josm:

fix #20310 - ImageImporter improvements (patch by Bjoeni)

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. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.