D21401: Use native dialog overwrite check

2019-05-28 Thread Nathaniel Graham 
ngraham added a comment.


  So nice!

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D21401

To: meven, ngraham, #frameworks, #kate, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, 
bruns, demsking, sars, dhaumann

D21401: Use native dialog overwrite check

2019-05-27 Thread Méven Car 
This revision was automatically updated to reflect the committed changes.
Closed by commit R39:fb2ea92822d2: Use native dialog overwrite check (authored 
by meven).

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21401?vs=58708=58710

REVISION DETAIL
  https://phabricator.kde.org/D21401

AFFECTED FILES
  src/document/katedocument.cpp
  src/document/katedocument.h

To: meven, ngraham, #frameworks, #kate, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, 
bruns, demsking, sars, dhaumann

D21401: Use native dialog overwrite check

2019-05-27 Thread Méven Car 
meven added a comment.


  Good part is that the overwrite check dialog will now be a native one analog 
to other apps instead of a KMessageBox, providing consistency outside of KDE.

REPOSITORY
  R39 KTextEditor

BRANCH
  arcpatch-D21401

REVISION DETAIL
  https://phabricator.kde.org/D21401

To: meven, ngraham, #frameworks, #kate, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, 
bruns, demsking, sars, dhaumann

D21401: Use native dialog overwrite check

2019-05-27 Thread Méven Car 
meven updated this revision to Diff 58708.
meven added a comment.


  Remove decl of removed function checkOverwrite

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21401?vs=58646=58708

BRANCH
  arcpatch-D21401

REVISION DETAIL
  https://phabricator.kde.org/D21401

AFFECTED FILES
  src/document/katedocument.cpp
  src/document/katedocument.h

To: meven, ngraham, #frameworks, #kate, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, 
bruns, demsking, sars, dhaumann

D21401: Use native dialog overwrite check

2019-05-26 Thread Christoph Cullmann 
cullmann added a comment.


  I think this is fine then, please remove the decl. of bool 
KTextEditor::DocumentPrivate::checkOverwrite(QUrl u, QWidget *parent), too and 
push this, thanks!

REPOSITORY
  R39 KTextEditor

BRANCH
  native-overwrite-check

REVISION DETAIL
  https://phabricator.kde.org/D21401

To: meven, ngraham, #frameworks, #kate, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, 
bruns, demsking, sars, dhaumann

D21401: Use native dialog overwrite check

2019-05-26 Thread Méven Car 
meven added a comment.


  In D21401#470121 , @ngraham wrote:
  
  > This will work fine for a KDE environment where we know that the file 
dialog correctly handles the overwrite use case. Do we know that the GTK file 
dialog does the same for when Kate is run on GNOME, XFCE, or MATE?
  
  
  I believe it will be fine. 
https://developer.gnome.org/gtk3/stable/GtkFileChooserDialog.html 
GtkFileChooserDialog has a `gtk_file_chooser_set_do_overwrite_confirmation` 
(since 2005's gtk 2.8) so I would expect QFileDialog to use this feature.
  So given GNOME, XFCE and MATE uses gtk >= 2.8, I don't foresee an issue on 
those DE.
  Also bug https://bugreports.qt.io/browse/QTBUG-11539 mentions it works under 
GNOME at least.
  
  On the QFileDialog side, I wish it would be better documented : 
https://doc.qt.io/archives/qt-4.8/qfiledialog.html#details mentioned GNOME and 
KDE but not anymore in qt5 stable documentation 
https://doc.qt.io/qt-5/qfiledialog.html#details

REPOSITORY
  R39 KTextEditor

BRANCH
  native-overwrite-check

REVISION DETAIL
  https://phabricator.kde.org/D21401

To: meven, ngraham, #frameworks, #kate, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, 
bruns, demsking, sars, dhaumann

D21401: Use native dialog overwrite check

2019-05-25 Thread Nathaniel Graham 
ngraham added a comment.


  This will work fine for a KDE environment where we know that the file dialog 
correctly handles the overwrite use case. Do we know that the GTK file dialog 
does the same for when Kate is run on GNOME, XFCE, or MATE?

REPOSITORY
  R39 KTextEditor

BRANCH
  native-overwrite-check

REVISION DETAIL
  https://phabricator.kde.org/D21401

To: meven, ngraham, #frameworks, #kate, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, 
bruns, demsking, sars, dhaumann

D21401: Use native dialog overwrite check

2019-05-25 Thread Christoph Cullmann 
cullmann added a comment.


  checkOverwrite decl. should be removed, too.

REPOSITORY
  R39 KTextEditor

BRANCH
  native-overwrite-check

REVISION DETAIL
  https://phabricator.kde.org/D21401

To: meven, ngraham, #frameworks, #kate, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, 
bruns, demsking, sars, dhaumann

D21401: Use native dialog overwrite check

2019-05-25 Thread Christoph Cullmann 
cullmann accepted this revision.
cullmann added a comment.
This revision is now accepted and ready to land.


  Sounds reasonable.

REPOSITORY
  R39 KTextEditor

BRANCH
  native-overwrite-check

REVISION DETAIL
  https://phabricator.kde.org/D21401

To: meven, ngraham, #frameworks, #kate, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, 
bruns, demsking, sars, dhaumann

D21401: Use native dialog overwrite check

2019-05-25 Thread Méven Car 
meven added a task: T8552: Polish Open/Save dialogs.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D21401

To: meven, ngraham, #frameworks, #kate
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D21401: Use native dialog overwrite check

2019-05-25 Thread Méven Car 
meven created this revision.
meven added reviewers: ngraham, Frameworks, Kate.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
meven requested review of this revision.

REVISION SUMMARY
  Past review https://git.reviewboard.kde.org/r/128452/ avoided double 
overwrite check by disabling the dialog one, it should have been the opposite, 
reusing QFileDialog.

REPOSITORY
  R39 KTextEditor

BRANCH
  native-overwrite-check

REVISION DETAIL
  https://phabricator.kde.org/D21401

AFFECTED FILES
  src/document/katedocument.cpp

To: meven, ngraham, #frameworks, #kate
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann