This PR adds an option that moves the warning from a popup to the status window when a project file is unwritable. Potentially addresses #2863. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/2953
-- Commit Summary --
* <a href="https://github.com/geany/geany/pull/2953/commits/7a2c1c21e2ddb4fabc389f1bbb1b65dd64e779fc">Add option to use status window to warn about unwritable project files</a>
-- File Changes --
M doc/geany.txt (2) M src/plugindata.h (2) M src/project.c (12) M src/ui_utils.c (2) M src/ui_utils.h (2)
-- Patch Links --
https://github.com/geany/geany/pull/2953.patch https://github.com/geany/geany/pull/2953.diff
@xiota pushed 1 commit.
e8203d3de1aa9bd5262bec39ac6f5c2132512701 include "msgwindow.h"
@xiota pushed 1 commit.
9963d03f0d3ef1c6e18c0b5115ac535f0dee8c34 How many ways are there to call a dialog box?
Does `ui_set_statusbar` send the message only to the status bar or also to the status window?
@xiota pushed 1 commit.
ff0ebcff50b7e433007ea427ce394720bf64c1df alignment
@xiota pushed 1 commit.
bf82f53adb156424b24823e67aa409eacedc5f70 Fix msgwin_status_add use
@xiota pushed 1 commit.
5e745672a73e66c39748c8f51e06b80474f34884 Fix previous fix
@xiota pushed 1 commit.
1a292d1fb295841a91624685ebec0ccbbf530901 Revert changes to translatable strings
@eht16 commented on this pull request.
@@ -2542,6 +2542,8 @@ msgwin_messages_visible Whether to show the Messages tab in the t
Messages Window msgwin_scribble_visible Whether to show the Scribble tab in the true immediately Messages Window +warn_on_project_close Whether to show a warning when opening true immediately
The option name and description doesn't match the rest of the feature. I guess this is a copy&paste mistake from #2949.
@eht16 requested changes on this pull request.
I'm wondering if this is really necessary. Errors on saving files should be noticed by users, usually one probably wants to fix the cause instead of ignoring it.
Yes, I noticed the use case in #2863 but I'm not sure whether this is worth a new option and handling it at all.
@xiota pushed 1 commit.
040107e1731b36657f532f68de2574d7410f67db Add option to use status window to warn about unwritable project files
@xiota commented on this pull request.
@@ -2542,6 +2542,8 @@ msgwin_messages_visible Whether to show the Messages tab in the t
Messages Window msgwin_scribble_visible Whether to show the Scribble tab in the true immediately Messages Window +warn_on_project_close Whether to show a warning when opening true immediately
Possibly copy/paste. Possibly I got confused by too many git branches. I've rebased and fixed this (maybe, if I didn't get confused again).
I'm wondering if this is really necessary. Errors on saving files should be noticed by users, usually one probably wants to fix the cause instead of ignoring it.
If this were for files being edited, I'd agree. But this is for project files. Since Geany functionality otherwise remains intact, this is more of a "warning" than an "error".
Geany also *frequently* saves project files, even when no changes have been made. This can lead to dozens of popups. If the warning cannot be redirected to the status window, it would be reasonable to change the warning behavior to show at most one popup per project file per session.
When I open the plugin manager, Geany sends a number oferror messages to stderr. Imagine if even just the "critical" errors created pop-ups? ``` (geany:5239): Gtk-WARNING **: 09:57:22.467: Negative content width -17 (allocation 1, extents 9x9) while allocating gadget (node entry, owner GtkEntry)
(geany:5239): Gtk-WARNING **: 09:57:22.467: Negative content height -1 (allocation 1, extents 1x1) while allocating gadget (node entry, owner GtkEntry)
(geany:5239): Gtk-WARNING **: 09:57:22.467: Negative content width -1 (allocation 1, extents 1x1) while allocating gadget (node scrolledwindow, owner GtkScrolledWindow)
(geany:5239): Gtk-WARNING **: 09:57:22.467: Negative content height -1 (allocation 1, extents 1x1) while allocating gadget (node scrolledwindow, owner GtkScrolledWindow)
(geany:5239): Gtk-WARNING **: 09:57:22.467: Negative content width -9 (allocation 1, extents 5x5) while allocating gadget (node scrollbar, owner GtkScrollbar)
(geany:5239): Gtk-CRITICAL **: 09:57:22.467: gtk_box_gadget_distribute: assertion 'size >= 0' failed in GtkScrollbar
(geany:5239): Gtk-CRITICAL **: 09:57:22.467: gtk_widget_get_preferred_width_for_height: assertion 'height >= 0' failed
(geany:5239): Gtk-WARNING **: 09:57:22.467: gtk_widget_size_allocate(): attempt to allocate widget with width 16 and height -29
(geany:5239): Gtk-WARNING **: 09:57:22.467: Negative content height -9 (allocation 1, extents 5x5) while allocating gadget (node scrollbar, owner GtkScrollbar)
(geany:5239): Gtk-CRITICAL **: 09:57:22.467: gtk_box_gadget_distribute: assertion 'size >= 0' failed in GtkScrollbar ```
I noticed the use case in #2863 but I'm not sure whether this is worth a new option and handling it at all.
Geany previously allowed users to have read-only project files without nagging them. According to reasoning used against me in another PR, the change to showing popup warnings represents removal of capability without fully considering use cases. There is even a known use case with at least one user who is affected. (While for my other PR, there is no known use case or affected user.)
Rather than handling each warning separately, a general warning function could be used that's passed a parameter that sends the warning to a popup, the status window, stderr, etc. If such a function already exists, then the project warnings could be modified to use them, rather than unconditionally using popups.
I'm wondering if this is really necessary. Errors on saving files should be noticed by users, usually one probably wants to fix the cause instead of ignoring it.
If this were for files being edited, I'd agree. But this is for project files. Since Geany functionality otherwise remains intact, this is more of a "warning" than an "error".
Geany also _frequently_ saves project files, even when no changes have been made. This can lead to dozens of popups.
The idea the popups is to inform the user that there is something wrong with the project file and that Geany could not write it. The user should fix the problem.
When I open the plugin manager, Geany sends a number of messages to stderr. Imagine if even just the "critical" ones created pop-ups?
(geany:5239): Gtk-WARNING **: 09:57:22.467: Negative content width -17 (allocation 1, extents 9x9) while allocating gadget (node entry, owner GtkEntry) [...]
These are not messages sent from Geany targetted to the user to notify her/him about a problem relevant for her/him. These are warnings from GTK and rather targetted to developers. But why these messages appear and how to fix them is a whole another story (and there exists already an issue for).
I noticed the use case in #2863 but I'm not sure whether this is worth a new option and handling it at all.
Geany previously allowed users to have read-only project files without nagging them.
I doubt that this is true. At least it was no intended feature, this contradicts the whole idea of project files. It might be that it worked somehow but I still don't consider this a valid, common use case.
The user should fix the problem.
Using file permissions to prevent unwanted changes to files has a history that is at least as old as file permissions themselves. It should be the user's decision whether programs can modify their files. While notifying the user of the "problem" is reasonable, nagging them with dozens of popups is not. After the user signals knowledge of the issue by either dismissing the popup once or checking an option to disable further popups, whether they "lose" any data is on them.
I doubt that this is true.
I am taking the issue reports at face value. They state that there were previously no warning popups. Then the warning popups were added.
this contradicts the whole idea of project files. It might be that it worked somehow but I still don't consider this a valid, common use case.
It contradicts *your* (and others') idea of project files. But at least one user wants a static project file to be able to always open a specific set of files.
I have opened this PR because it should be up to the user, not Geany devs, whether their files are marked read only. People who like the popups are free to leave the warnings in place.
Having settings (which should only save when the user changes something) separate from sessions (which change every time the active tab changes) would pretty much allow for frozen settings files.
Initial work for geany.conf is ongoing in the `session-split` branch and the same solution is then intended to be applied to project settings/sessions.
Perhaps helping to advance that would be better than making more individual changes to project handling.
Having settings (which should only save when the user changes something) separate from sessions (which change every time the active tab changes) would pretty much allow for frozen settings files.
* That would not address the case in which the user wants the *session* frozen. * That also does not address respecting user file permission settings or notification preferences (popup vs status window).
As @eht16 said above and on #2863 it is not possible to support all use-cases. Each case requires code which has to be maintained, and added complication adds support required by users. Geany is maintained by volunteers only who are doing it in their own time. It is not required that they accept every request for a feature or that a feature be implemented exactly the way it is requested if experienced project contributors suggest an alternative.
It is great that you want to contribute, but it would be better to ask first before making the effort to implement. Feature requests that are considered reasonable are mostly marked with the "enhancement" label. If its not marked either nobody has looked at it, or there is an issue with adding it, so definitely ask first.
Related #2304. codebrainz states in a [comment](https://github.com/geany/geany/issues/2304#issuecomment-531600791):
I find the file permissions approach superior to some in-app thing...
That is what this PR attempts to allow.
When write permissions are removed from `geany.conf`, there are no error messages or popups. Project files and `geany.conf` both store config and session info. Why are read-only project files something that the user *must* "correct", but read-only `geany.conf` is not even worth notifying the user about?
When write permissions are removed from geany.conf, there are no popups or messages (not even in the status window). Project files and geany.conf both store config and session info. Why are read-only project files something that the user must "correct", but read-only geany.conf is not even worth notifying the user about?
Good catch, `keyfile.c::configuration_save()` does not check the return value from `utils.c::utils_write_file()` but `project.c::write_config()` does. At first glance I would call it a bug. Should have its own issue.
It may be as simple as "someone wrote it that way" but possibly its intentional, checking back in git history to find when one or the other was added/changed might be enlightening, or one of the long term contributors may remember why.
@xiota pushed 1 commit.
3c11c793eca3cc6acef7fe7d40ac23958bb7dac8 Add option to use status window to warn about unwritable project files
Force push to rebase and guarantee no conflicts with recent session-split commit.
github-comments@lists.geany.org