Opened 3 years ago
Last modified 20 months ago
#21931 new defect
[Patch] .nmea/.pos/.wpt files always ask to be saved
Reported by: | Bjoeni | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | Core | Version: | tested |
Keywords: | nmea, pos, wpt, gpx, save | Cc: |
Description
Files that are displayed as GPX layers but are not loaded from a *.gpx file always ask to be saved when removing them, even when nothing was changed.
To my knowledge this currently affects the formats
- *.nmea
- *.pos
- *.wpt
- possibly some other formats and/or plugins
Since r18287 a (*) is displayed next to the unsaved layers, but this issue appears to have existed before that.
Kind of related: When saving such a layer it says "no exporter found". I think the filepath should automatically be changed to *.gpx so that they can be saved, however that must only happen when actually saving: As long as the file is unchanged the original file can be referenced while saving a session.
I'll probably wait until the patches #19219, #21813, #21922, #21923 are committed before creating a patch for this ticket as it might conflict.
Attachments (2)
Change History (11)
comment:1 by , 2 years ago
Milestone: | → 22.11 |
---|---|
Owner: | changed from | to
Status: | assigned → new |
Summary: | .nmea/.pos/.wpt files always ask to be saved → [Patch] .nmea/.pos/.wpt files always ask to be saved |
by , 2 years ago
Attachment: | 21931.patch added |
---|
follow-up: 9 comment:5 by , 2 years ago
@Bjoeni: What should happen in GpxExporter#exportData if quiet
is true
and the file exists?
comment:6 by , 2 years ago
Also, when asking the user if they want to overwrite (Would you like to overwrite the existing file
), why not use ConditionalOptionaPaneUtil
?
Anyway, I'll go ahead and upload some tests (which use the current ExtendedDialog
method).
comment:8 by , 22 months ago
Milestone: | 23.03 |
---|
I'm dropping the milestone since I haven't gotten a response for comment:5. I also don't like extending File
to avoid asking about overwriting a file, and I don't think the question in GpxExporter
is necessary -- it should be covered by the createAndOpenSaveFileChooser
. If it is actually necessary, the code path that causes the need should probably have the same behavior.
comment:9 by , 20 months ago
Hey Taylor,
sorry about not getting back to you earlier.
@Bjoeni: What should happen in GpxExporter#exportData if
quiet
istrue
and the file exists?
The user will still be asked about overwriting the file. The quiet option only refers to regular prompts (e.g. GPX attributes), not warnings. I agree that could probably be named better or the Javadoc could be updated, I think I introduced that with r15496. So it basically only exists for the GPX attributes dialog.
Also, when asking the user if they want to overwrite (
Would you like to overwrite the existing file
), why not useConditionalOptionaPaneUtil
?
I didn't want to change the existing behaviour. That was just moved from SaveAction
to GpxExporter
. Also I like the way the dialog is at the moment so I didn't really see a reason to change it.
I also don't like extending
File
to avoid asking about overwriting a file,
Tbh I don't really see an issue with that. It was the easiest way of accomplishing it in a clean matter without redesigning the whole SaveActionBase
which would have opened a whole other can of worms. Saving the file is handled by the SaveActions and writing it is handled by the Exporters.
and I don't think the question in
GpxExporter
is necessary -- it should be covered by thecreateAndOpenSaveFileChooser
.
That doesn't work for two reasons: The question must also appear when saving an existing file that was created by a different software (e.g. a file created by a GPS device, which was modified inside JOSM), because we can't guarantee that all information will always be kept. So it must be shown even when the "Save As" dialog is never shown.
Also I didn't want to change the existing behaviour. So more general: The dialog in GpxExporter
is supposed to warn the user that his original file will be modified, because that might not be expected and depending on the original creator
software some information might be lost.
The prompt in the "Save As" dialog on the other hand is supposed to tell the user "hey man, you just clicked on another file that might be completely different from the one you're trying to save. Do you really want to overwrite that?".
If it is actually necessary, the code path that causes the need should probably have the same behavior.
Once again this is pretty GPX-specific behaviour. It depends on which software it was written by if the user will be asked (no need to ask when it was written by JOSM). That's actually the reason why I moved it from SaveAction
to `GpxExporter´, because the SaveAction should be quite abstract and not contain any logic related to specific layer types. See the changelog in comment:1.
Patch:
NmeaReader
OziWptReader
RtkLibPosReader
GpxLayer
that are not loaded from .gpx files (i.e. the formats above)GpxExporter
*GpxExporter#exportData()
*) that was required because overwriting must be checked for after the correct file path is known. Also it makes much more sense there than a specific rule for
GpxLayer
in the rather abstractSaveAction
.Note: Currently the extension .gpx will just be added to the current file name. This means
test.nmea
will be saved astest.nmea.gpx
. I think this makes sense as it indicates the original file format, but could obviously be changed.