Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

geany.conf not checked for successful save. #2993

Open
elextr opened this issue Nov 9, 2021 · 7 comments
Open

geany.conf not checked for successful save. #2993

elextr opened this issue Nov 9, 2021 · 7 comments

Comments

@elextr
Copy link
Member

elextr commented Nov 9, 2021

From #2953 (comment) thanks @xiota

As noted in the comment above, the return from utils_write_file() of geany.conf is not checked, but the return is checked for write of a project file.

As pointed out this seems inconsistent, is it deliberate and should it be changed?

@xiota
Copy link
Contributor

xiota commented Nov 9, 2021

My opinion... users should be notified (not nagged) that there is a potential problem (status window during normal operation, stderr on close). Then they can decide whether to "fix" the problem.

The "problem" may be intentional. Setting file permissions has a long history of use to prevent changes to files.

The same approach should apply to project files.

@elextr
Copy link
Member Author

elextr commented Nov 10, 2021

As @eht16 said elsewhere, just because something happens to work that way, doesn't mean its intended to.

After all by that measure there would never be any bugs, because buggy behaviour is the way the software works (hmmmmmmmm, maybe you are onto something 😄).

@xiota
Copy link
Contributor

xiota commented Nov 19, 2021

@elextr @eht16 Would either of you object to unifying the read-only warning message behavior of project files, geany.conf, and (eventually) session.conf to use a popup by default with the option to redirect to the status window (and/or stderr)? This could be done by having all three call the same error message function.

Users would be notified of potential problems with read-only files, including cases where they are not currently notified, while also being free to mark file permissions as they see fit.

@elextr
Copy link
Member Author

elextr commented Nov 20, 2021

@xiota IIUC the current behaviour is there are messages for "read only", "do you want to overwrite" and maybe other conditions when a project file is first created, because those conditions are explicitly checked, but they are not checked again.

So from then on its a message meaning "project save failed" that can be generated for many reasons, not just permissions. And as I said above, saving geany.conf actually doesn't check for failure, so making it do so is a "good thing".

I would advise patience (yeah, I'm not good at it either) until the session-split branch is merged to avoid conflicts and re-bases and then you can do all three files at once.

Then converting the save of all three prefs files to test the return code from utils_write_file() and use a common dialog in dialogs.c if it fails.

That common dialog can have a "Show in status bar and window instead" checkbox, but its hard to see how that can be saved in prefs when it can only be selected when saving prefs has failed 😁 so it would show at least once per session. But that seems useful to me. Sadly utils_write_file() does not return a message string so it can be shown in the dialog, maybe a new version that does is needed.

One thing is that although there is only ever one geany.conf and session.conf file per session, projects can be opened and closed. Just because one project fails due to deliberate permissions does not mean the next one opened is the same, so maybe the flag should be reset when projects change. That kind of suggests a separate flag for preferences/session and for projects.

@elextr
Copy link
Member Author

elextr commented Nov 20, 2021

Or alternatively the warning that prefs or project failed to save could be provided by a gtk_info_bar which shows at the top of the edit pane but is not modal and doesn't interrupt the user editing like a dialog does (an example is the thing that appears when a file on disk changes) so its better for an error that can occur at various different times. Info bars can have buttons, so it could have one to "Only show in status bar and status window from now on" as well as "Retry" and "Cancel".

@xiota
Copy link
Contributor

xiota commented Nov 20, 2021

So from then on its a message meaning "project save failed" that can be generated for many reasons, not just permissions.

I refer to permissions and read-only files because those are the most common conditions that produce popups. The messages can be produced several times a minute, even seconds apart, depending on how often the user saves files, switches documents, or performs other activities that prompt project saving. This makes Geany unusable.

And as I said above, saving geany.conf actually doesn't check for failure, so making it do so is a "good thing".

You seemed averse to changing the project popups messages. I suggested unifying config file behavior as a compromise to add checking for files that aren't checked, while reducing the intrusiveness of the current checks.

I would advise patience (yeah, I'm not good at it either) until the session-split branch is merged to avoid conflicts and re-bases and then you can do all three files at once.

The session-split branch makes managing preferences more complicated than it already is. In the long run, it would be better to migrate to the stash system. The current near-duplicate save/load code would become a single "add". Then the stash system could be extended with session support. The add/save/load functions could take a bool or have session variants. Since the stash system uses pointers to the preference variables, no other changes would be required.

so it would show at least once per session.

Various preferences.

Just because one project fails due to deliberate permissions does not mean the next one opened is the same, so maybe the flag should be reset when projects change.

The changes I've suggested do not eliminate user notification. It makes them less intrusive. Users would be able to more easily ignore them. But that's up to them.

That kind of suggests a separate flag for preferences/session and for projects.

That would be fine with me.

Or alternatively the warning that prefs or project failed to save could be provided by a gtk_info_bar which shows at the top of the edit pane but is not modal and doesn't interrupt the user editing like a dialog does...

That could also work.

@elextr
Copy link
Member Author

elextr commented Nov 21, 2021

So from then on its a message meaning "project save failed" that can be generated for many reasons, not just permissions.

I refer to permissions and read-only files because those are the most common conditions that produce popups. The messages can be produced several times a minute, even seconds apart, depending on how often the user saves files, switches documents, or performs other activities that prompt project saving. This makes Geany unusable.

Which is why the infobar is a better idea, the user is informed, but doesn't immediately have to dismiss it, they can keep working, or go investigate why it failed, and as I said it can have a "show on status bar and status window only" button so if they intend the save to fail they can use that.

The session-split branch makes managing preferences more complicated than it already is.

Yes, thats (part of) why its taken literally decades to get to the current branch from when the idea was first floated.

But as the announcer said "you ain't seen nothing yet" wait until project files is split too 😈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants