martinkostolny updated this revision to Diff 12251. martinkostolny added a comment.
Thanks a lot for all the thoughts and suggestions! I tried to work them in, but I need help with some of them. I'm struggling with the atomic rename. I still find Christoph's suggestion (save to temp file and then move it in helper) a good option to stick with. But then we are in many cases unable to atomically move the tempfile because tempfile is usually in tmpfs (RAM) -> different device then target file to save. So we can't even use std::rename for this. Exist & remove & rename are racy so I went for copying to the QSaveFile and then commit. It's not exactly a one-liner, is there a better way? I may very well be on a wrong path here. About the event loop evil. I agree it should be re-written with QT connects and I don't mind digging in the code. But if I understand the existing code correctly, I'd also have to rewrite at least KateBuffer::saveFile(), DocumentPrivate::saveFile() and DocumentPrivate::documentSaveCopyAs(). I'd like to suggest doing all this in a separate diff/review. Did I miss something else? REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4847?vs=12185&id=12251 REVISION DETAIL https://phabricator.kde.org/D4847 AFFECTED FILES autotests/src/katetextbuffertest.cpp autotests/src/katetextbuffertest.h src/CMakeLists.txt src/buffer/katesecuretextbuffer.cpp src/buffer/katesecuretextbuffer_p.h src/buffer/katetextbuffer.cpp src/buffer/katetextbuffer.h src/buffer/org.kde.ktexteditor.katetextbuffer.actions To: martinkostolny, dhaumann, #ktexteditor Cc: dfaure, anthonyfieroni, cullmann, ltoscano, dhaumann, graesslin, davidedmundson, palant, kwrite-devel, #frameworks, head7, kfunk, sars
