Hello Geany Developers,
I’m a software developer who has had fun using Geany for my own projects, so thank you for all the hard work you guys have been doing - I love it! Hopefully this is the right place to post this - if there is somewhere else I should post this additionally and/or instead let me know.
When playing around with modifying plugins I found a potential issue with document closing. The ‘document-close’ signal only supplies plugins with a document that has the ‘changed’ flag set if one document is closed, e.g. a user hit the 'x' on the UI document tab, but not when all documents are being closed, e.g. when quitting Geany, regardless of whether the ‘document’ has changes. The cause for this behavior appears to be the 'document_account_for_saved' function in document.c which sets the 'changed' flag to FALSE when closing all documents. I've attached a patch to this email that could address this. At the bottom of this email I've described the approach I took with this patch. If the patch doesn’t look good let me know what I can do to fix it, or if this behavior is expected, would love suggestions on how a plugin should be able to get this information when a document is being closed.
I'm running Geany by installing it from source from the most recent version: 1.34., but the behavior I see is caused by functions and code which appear to have been around for awhile so I don't think this would be much different for previous versions.
Thank you for any feedback you guys can give. Again really love the Geany project, and I’m glad you’ve made it possible to have fun coding without requiring shelling out massive amounts of money to use a really nice light-weight editor.
Jason
The approach in the patch is simple – remove the code that modifies the ‘changed’ flag in 'document_account_for_saved', and add a check in the ‘remove_page’ function for the ‘main_status.closing_all’ to avoid prompting the user when closing all documents. This flag gets set during ‘force_close_all’ which is only called in ‘document_close_all’, so I couldn’t find anything wrong with this approach – let me know if that is not the case.
The only other change that was needed is in how ‘main_quit’ handles closing all documents to ensure the current behavior behavior isn’t changed when closing Geany, especially since it also calls ‘document_account_for_unsaved’. The approach I took to this was to remove the ‘document_account_for_unsaved’ out of the ‘main_quit’ function since this will be called with ‘project_close’ or ‘document_close_all’ in 'do_main_quit', and change the ‘do_main_quit’ function to use the return value from ‘project_close’ and/or ‘document_close_all’ to allow aborting the procedure if the user canceled and pass this back up to the ‘main_quit’ , or return TRUE at the end of the function call otherwise.
Hi,
The best way of providing for changes would be to make a pull request on Github. Then they will at least be CI checked.
Cheers Lex
On 13 May 2018 at 16:55, Jason Cumbie jscumbie@gmail.com wrote:
Hello Geany Developers,
I’m a software developer who has had fun using Geany for my own projects, so thank you for all the hard work you guys have been doing
- I love it! Hopefully this is the right place to post this - if there
is somewhere else I should post this additionally and/or instead let me know.
When playing around with modifying plugins I found a potential issue with document closing. The ‘document-close’ signal only supplies plugins with a document that has the ‘changed’ flag set if one document is closed, e.g. a user hit the 'x' on the UI document tab, but not when all documents are being closed, e.g. when quitting Geany, regardless of whether the ‘document’ has changes. The cause for this behavior appears to be the 'document_account_for_saved' function in document.c which sets the 'changed' flag to FALSE when closing all documents. I've attached a patch to this email that could address this. At the bottom of this email I've described the approach I took with this patch. If the patch doesn’t look good let me know what I can do to fix it, or if this behavior is expected, would love suggestions on how a plugin should be able to get this information when a document is being closed.
I'm running Geany by installing it from source from the most recent version: 1.34., but the behavior I see is caused by functions and code which appear to have been around for awhile so I don't think this would be much different for previous versions.
Thank you for any feedback you guys can give. Again really love the Geany project, and I’m glad you’ve made it possible to have fun coding without requiring shelling out massive amounts of money to use a really nice light-weight editor.
Jason
The approach in the patch is simple – remove the code that modifies the ‘changed’ flag in 'document_account_for_saved', and add a check in the ‘remove_page’ function for the ‘main_status.closing_all’ to avoid prompting the user when closing all documents. This flag gets set during ‘force_close_all’ which is only called in ‘document_close_all’, so I couldn’t find anything wrong with this approach – let me know if that is not the case.
The only other change that was needed is in how ‘main_quit’ handles closing all documents to ensure the current behavior behavior isn’t changed when closing Geany, especially since it also calls ‘document_account_for_unsaved’. The approach I took to this was to remove the ‘document_account_for_unsaved’ out of the ‘main_quit’ function since this will be called with ‘project_close’ or ‘document_close_all’ in 'do_main_quit', and change the ‘do_main_quit’ function to use the return value from ‘project_close’ and/or ‘document_close_all’ to allow aborting the procedure if the user canceled and pass this back up to the ‘main_quit’ , or return TRUE at the end of the function call otherwise.
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
On 13 May 2018 at 17:35, Lex Trotman elextr@gmail.com wrote:
Hi,
The best way of providing for changes would be to make a pull request on Github. Then they will at least be CI checked.
Also on Github you can point to parts of the source which support your explanation, I can't immediately see where having "changed" false stops the signal being emitted.
Cheers Lex
Cheers Lex
On 13 May 2018 at 16:55, Jason Cumbie jscumbie@gmail.com wrote:
Hello Geany Developers,
I’m a software developer who has had fun using Geany for my own projects, so thank you for all the hard work you guys have been doing
- I love it! Hopefully this is the right place to post this - if there
is somewhere else I should post this additionally and/or instead let me know.
When playing around with modifying plugins I found a potential issue with document closing. The ‘document-close’ signal only supplies plugins with a document that has the ‘changed’ flag set if one document is closed, e.g. a user hit the 'x' on the UI document tab, but not when all documents are being closed, e.g. when quitting Geany, regardless of whether the ‘document’ has changes. The cause for this behavior appears to be the 'document_account_for_saved' function in document.c which sets the 'changed' flag to FALSE when closing all documents. I've attached a patch to this email that could address this. At the bottom of this email I've described the approach I took with this patch. If the patch doesn’t look good let me know what I can do to fix it, or if this behavior is expected, would love suggestions on how a plugin should be able to get this information when a document is being closed.
I'm running Geany by installing it from source from the most recent version: 1.34., but the behavior I see is caused by functions and code which appear to have been around for awhile so I don't think this would be much different for previous versions.
Thank you for any feedback you guys can give. Again really love the Geany project, and I’m glad you’ve made it possible to have fun coding without requiring shelling out massive amounts of money to use a really nice light-weight editor.
Jason
The approach in the patch is simple – remove the code that modifies the ‘changed’ flag in 'document_account_for_saved', and add a check in the ‘remove_page’ function for the ‘main_status.closing_all’ to avoid prompting the user when closing all documents. This flag gets set during ‘force_close_all’ which is only called in ‘document_close_all’, so I couldn’t find anything wrong with this approach – let me know if that is not the case.
The only other change that was needed is in how ‘main_quit’ handles closing all documents to ensure the current behavior behavior isn’t changed when closing Geany, especially since it also calls ‘document_account_for_unsaved’. The approach I took to this was to remove the ‘document_account_for_unsaved’ out of the ‘main_quit’ function since this will be called with ‘project_close’ or ‘document_close_all’ in 'do_main_quit', and change the ‘do_main_quit’ function to use the return value from ‘project_close’ and/or ‘document_close_all’ to allow aborting the procedure if the user canceled and pass this back up to the ‘main_quit’ , or return TRUE at the end of the function call otherwise.
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel