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

Hide overwrite button for readonly documents. #3142

Closed
wants to merge 1 commit into from
Closed

Hide overwrite button for readonly documents. #3142

wants to merge 1 commit into from

Conversation

madpilot78
Copy link
Contributor

I have noticed that even when I mark a document "readonly" in the UI (for example via the document menu), when Geany detects a file change, the requester proposes me to overwrite it anyway.

This looks wrong, I mark the file read only because I want to avoid writing to it by mistake.

I often open the same file in two windows and mark it read only in one of those, I find this better than the split window (which I also use sometimes). I happened to overwrite a file by mistake due to a stray click on the overwrite button, so I decided to create this patch.

@elextr
Copy link
Member

elextr commented Mar 28, 2022

Being able to overwrite the changed file with an unchanged document (ensured because its read-only) is also an important use-case, so blanket removal of the capability is not appropriate.

Adding an additional option to "Ignore file changes" per document would be a better solution. I would think its best for it to be separate from the file changed notification (in Document menu for eg) so you can set it at the same time you set read-only and never see the notification.

This is one of many problems with running multiple instances, have you closed them in the wrong order and saved the wrong session yet?

The best solution would be to teach Geany to support multiple edit widgets properly, allowing multiple windows from one instance and fixing the split window problems, but investigations show thats a fair amount of work and somebodys got to do it.

@madpilot78
Copy link
Contributor Author

I understand not wanting to remove the functionality, but also not getting notified of the on disk file having changed does not look correct. I interpreted "readonly" as an indication that the file would never be written to, not as "will not allow modifications in the buffer". With this new interpretation I should reassess the whole idea I had about how things should work. I assumed one meaning of readonly and adjusted the code to that.

With this new idea I'm not sure what is the best, maybe a global configuration option, specifying the expected behavior of readonly, or a new file property, which I'm not sure how to name (something like "never overwrite"), to specify the file should never be written to, but can be monitored for changes, completely orthogonal to the current readonly setting.

Since all these options require definitely more work than my simplistic approach, I'm not sure if I can find time to do such changes in the near future.

What would be the preferred/more acceptable behavior?

@elextr
Copy link
Member

elextr commented Mar 29, 2022

Its an important paradigm to grasp, a "file" is a bunch of bytes on a persistent storage medium, but no modern editor/IDE operates on the file, they operate on a memory structure variously called a "buffer", or as in Geany a "document", and will not touch the file except when you load or save the document. Thus we have a File menu and a Document menu, and Readonly is in the document menu, ie it applies to the document, not the file.

If the document has been modified and then is set readonly it can't be changed any more, but it can still be saved to the file, and it is always possible to save-as, even an unmodified readonly document and even to the original file name. Allowing the overwrite option on the change notification just matches these other file operations. So any "never write the file" option will have to be respected by all those operations, and there are other operations that save to the file (such as build commands), and the autosave plugin, and any other plugin that can save the document to the file. So it becomes complicated to actually implement a "never write to the file" option.

Note that when you open the file in two windows you have two copies of Geany running (instances in our terminology) and these do not know about each other. So if one saves to a file which another has in an open document, and therefore is monitoring for modifications, you will be notified. It seemed to me that an option removing the annoyance of notifications, along with its attendant "overwrite" button, would be relatively simple and remove the immediate risk for your multiple instance use-case.

@madpilot78
Copy link
Contributor Author

Thanks, actually most of that is now clear to me.

Actually when working with two instances, I find the notification useful, what I wanted to avoid was the "overwrite" button, because I wanted to be sure to write to file from only one window.

I understand that geany permits running in multiple windows by running two separate processes with no communication or sharing of data structures.

But all considered it means what I'm trying to obtain cannot really be obtained except by adding to geany a multi-window functionality where two separate windows editing the same file actually share the same document/buffer.

I don't know geany source base well enough to judge but implementing that, while interesting, looks like a complex task that could take months.

So, at present I'll just close this pull request, maybe I'll think about something else to suggest in the future.

@madpilot78 madpilot78 closed this Mar 29, 2022
@elextr
Copy link
Member

elextr commented Mar 29, 2022

with no communication or sharing of data structures.

Unless you start Geany with different -c directories multiple instances will share the preferences and session files, and both will write to those on close, hence my comment above that closing them in the wrong order can leave you with the wrong session.

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

Successfully merging this pull request may close these issues.

None yet

2 participants