ViewPrivate: Disable read-only mode action when doc is modified

Authored by loh.tar on Mar 4 2019, 1:47 PM.


Group Reviewers

It's somehow pointless to activate the read-only mode when the document
is already modified. However, after an save event becomes the action
available again.

Diff Detail

Lint Skipped
Unit Tests Skipped
loh.tar created this revision.Mar 4 2019, 1:47 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptMar 4 2019, 1:47 PM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
loh.tar requested review of this revision.Mar 4 2019, 1:47 PM

Hmm, two things:

  1. implementation wise: don't we have a problem here that the connect(document(), &KTextEditor::DocumentPrivate::modifiedChanged, [=] { m_toggleWriteLock->setEnabled(!doc()->isModified()); }); will not be unconnected if the view dies and we call on an invalid pointer?
  1. logic wise: why not allow people to set read only if it was modified? That avoids that you type something into the document by accident.
cullmann requested changes to this revision.Mar 4 2019, 6:41 PM

Thought about it bit more, actually like the "I can save even thought not modified" I think the "I can set read-only thought modified" is ok and actually no bug but a feature.
I see no merit in forbidding that (and actually would be even confused why this is disabled sometimes).

This revision now requires changes to proceed.Mar 4 2019, 6:41 PM
loh.tar added a subscriber: ngraham.Mar 4 2019, 7:43 PM

There is some reason why I came to this idea. ATM is that thought not so sharp present :-/
But it was related to the auto-reload patch and the lock icon in the bar.

@ngraham what do you think in general? Make a read only setting to an already modified file sense from the logic point, not because some strange use case?
Why not save the file with the changes you had done? Or reload to discard?

@ngraham Nate? Your opinion to the question?

In general I don't think it makes sense to be able to mark an already-modified-but-not-yet-saved document as read-only. It's already been written to!

So, then I assume you would be pro this patch.

I notice one behavior that would be against this patch but not pro the meaning of "Read Only Mode". When you set this setting and issue a save request you get the "Save as" dialog. Such behavior is good in any case. But such option, working as the current one, should then be called like, "Don't Overwrite File".

When you open as normal user a not writeable file, like /etc/fstab, I would somehow expect to see some warning and/or to enter the "Read Only Mode". But that's not the case. When you issue a save request you will be promted for admin rights, which is good in that example, no doubts.

Then exist the case that an "dangerous/odd" file, e.g. with too long lines, will set to "Read Only Mode". So far I remember is that also only be done to protect the orig file to be overwritten, but not for some hard needs which couldn't be handled by KTextEditor, it's more some odd protection against bad edit performance. So here is also a "Don't Overwrite File" setting more needful at the long end.

So, have have the feeling "Read Only Mode" should be renamed and this patch trashed, and the lock icon patch presumably too :-/

I actually would just keep the status quo here ;=)

I still think we should stay with status quo, can you drop this request?

loh.tar abandoned this revision.Apr 13 2019, 2:36 PM

To relieve the maintainer it's droped now.
But I still have the feeling there could be something improved at these "Read-Only Stuff"