aacid added a comment.
In https://phabricator.kde.org/D8642#167049, @rkflx wrote: > In https://phabricator.kde.org/D8642#167025, @aacid wrote: > > > > 1. [Ctrl] + [S] saves and is visible in Configure Shortcuts, but does not show up in the menu. > > > > Works just fine here. > > > To clarify, in the File menu, I only see Save As, but no entry for Save. Any special trick to make it show up? (I just used `make install` to `~/somedir` and set the paths as outlined here <https://community.kde.org/Guidelines_and_HOWTOs/Build_from_source#Set_up_the_runtime_environment>.) Ah, part.rc needs a version increase. Done in the branch. > > >>> 5. Adding Save to the toolbar, the button is not disabled if no changes have been made (initially, but also for undo). >> >> I don't agree in having the Save button in the toolbar by default, the toolbar is pretty full as it is now, and while it's true that we can save, i think the majority of people using Okular just use the viewer part. > > I fully agree, but that's not what I meant. "Adding" was about "When I manually add the button to the toolbar", otherwise I would have written "Please add <…>, because <reasons>". You may have missed the main part of the sentence, which was about the (non)disabled state. Ah, you want save to be only enabled when the file has been modified? I don't see this being a common pattern for the Save action , e.g. kate doesn't do that, libreoffice doesn't do that. > > >>> 7. "Do you want to save your changes or discard them?": Would be nice to mention what changed exactly ("Huh, I did not change //anything//!") because for PDFs this might not be obvious, e.g. by using a similar dialog to the one with the list you get when Saving a PNG, or just by changing the text. Also mention the filename. Suggestion (separate text for forms, if possible, to avoid the "and/or"): "Annotations and/or forms in the document "<filename>" have been modified. <linebreak>Do you want to save your changes or discard them?". >> >> Not sure this is doable, you really want a list of the 185 annotations you added to the document to be listed there? > > Good point. Just use my wording suggestion then (which does not imply having the list). Sincerely i don't see the need of saying "Annotations and/or forms", users are not that stupid. As mentioned in other comments i've already added the filename to the text. > > >>> 22. Wording: Remove linebreak and replace "and continue editing the document" with "to a supported storage format, if you want to continue to edit the document." >> >> "to a supported storage format" seems to technical, people would be scared because they don't know what that means, and in most of the cases it will hopefully it will be a pdf file and then the changes can just be saved fine wihtout the scary dialog about the okular archive format. > > Okay, my main motivation was more about rewording the last part which I stumbled upon. Could you change this to ", if you want to continue to edit the document."? Changed. > > > In https://phabricator.kde.org/D8642#167037, @aacid wrote: > >> > 23. Data loss on external change despite trying to save: Open, add annotation, change file externally, wait for warning, Save. Instead of overwriting, there is an error in the UI ("Could not open file:///…") and on the console ("The document hasn't been reloaded/swapped correctly"). After this, the file is corrupted but non-zero sized, i.e. Okular is not able to open it (MuPDF works, amazingly). Binary diff shows annot stream added at the end, perhaps some problem there? Note that Cancel and Save as with a different filename does work correctly. We do seem to have the data, but there is an issue saving to a file with the same name (not sure what this means in terms of file handles). >> >> I can't reproduce this, what do you mean by change file externally exactly? touch? edit it? remove it? copy something else over? ( i tried touch and my awesome pdf editing skills with vim) > > > Cannot check right now, IIRC this was with both `latexmk` and `cp somefile.pdf file_opened_in_okular.pdf` (comment if you still cannot reproduce and I will check in the evening). That's obviously not part of a regular workflow, but could happen by accident in reality. Yes, cp has this problem. This problem is on the other hand not new, if with the "old" version of okular you do the same, you will have exactly the same problem. I will try to fix it since you're right you can get data loss but it's nothing new and thus i don't think we should block the landing of this feature because of that. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D8642 To: aacid, mlaurent Cc: rkflx, lueck, mlaurent, michaelweghorn, ngraham, #okular, aacid
