The manual "Dynamic Word Wrap" config key is also the key used by the
generic config for this parameter, so config-backward-compatible change.
Details
Details
- Reviewers
loh.tar cullmann dhaumann - Group Reviewers
Kate - Commits
- R39:0611924df7b0: Store and fetch complete view config in and from session config
Saving & loading a KDevelop session now restores more of the view settings,
like which borders are shown.
Diff Detail
Diff Detail
- Repository
- R39 KTextEditor
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
I am a bit surprised this was not already done before? Did I miss something?
Settings like color scheme & zoom are not yet covered, but they might want to be restored as well fpr completeness, or? If someone changed them from the general settings, they might have had a reason, and that might still be present after session restore.
Comment Actions
Hmm, actually not sure why we didn't do that before.
I think this change makes sense.
Perhaps Dominik has some idea why we didn't do that in the past?
I myself think this change makes sense, but let's wait for Dominik's feedback before going forward.
Comment Actions
I guess we can give this a try. As I understand, though, with this patch you will never be able to use e.g. a global zoom once you change the zoom of a view. This was different before this patch.
Which brings me to another point: e.g. zoom should simply zoom all documents, so zoom should not be done per view imho. In particular, with this patch you can end up with a different font size for each view, which is certainly not what a user wants.
Comment Actions
Zoom is like all view stuff local, yes, I assume that is often not wanted.
But that is a orthogonal issue.
The same could be said for "dynamic word wrap", very seldom you want to set that for one view.
On the other side, for the global config, one has the settings dialog, same for "zoom", aka font size.
Comment Actions
When it comes to Zoom, I would only expect it adapts to Global zoom settings again only once if I have reset my manual zooming to Default again.
Though. seems Zoom is not covered by that config setting object... please not the "not" in "are not yet covered" in my first comment :)
So, time to push now? Bit unsure how to take your last comments.
Comment Actions
Seems this change has some sideeffects I did not experience when I played with this, but which are now uncovering as it hits people usinjg KF 5.70 :
config seems to write any settings, also the ones inherited from global defaults, as view-specific settings, and when reading them in again on session restart, they all become view-specific overrides, thus no longer influencable by global settings. Is that due to some other change elsewhere, or did I just completely miss this?
Possibly though this was mentioned before though:
If so, I did not get the message, as I was focussed on the property itself, "zoom", which is not covered by the settings handled here.
So, what to do? I would argue that revert would be the best option for now, as the sideeffects are more grave than what this patch tried to solve.
Comment Actions
I suggest to revert, and send a notification with the change to kde-distro-packager@kde.org to avoid that many users break their configuration.
Comment Actions
Hmm, right, didn't think about that :(
Guess if we want to have this, we need to improve the read/writeConfig functions.