Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
Merged #2859 into master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#event-5289023623
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@eht16 approved this pull request. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#pullrequestreview-752141777
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
Yeah I'm good with it too, only thing I wish was different was that this had the CTRL+SHIFT+R keybinding default but something else is there already -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#issuecomment-916592143
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@eht16 ok by me. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#issuecomment-907887445
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
Great, thanks for the update @Davidy22. If nobody objects, I'm going to merge this in a few days. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#issuecomment-907783633
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
alright i just pasted the changes in -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#issuecomment-899038992
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 pushed 1 commit. 43786d9f28a8880f68df4db51c1f1e02a662236d Docs changes -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2859/files/bf2b3176d19803b8a45f2e8d18794a26477a6ea8..43786d9f28a8880f68df4db51c1f1e02a662236d
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request. > @@ -3349,6 +3349,8 @@ Close Ctrl-W (C) > Closes the current fil Reload file Ctrl-R (C) Reloads the current file. +Reload allReloads all open files. Actually: "Reloads all open files. If the reload will not be 'undo'-able and changes that will be lost are detected (unsaved or saved) the reload will be confirmed, otherwise the reload will proceed without confirmation" -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#discussion_r688835234
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
I can't test ATM, but LGBI. Couple of comments for the doco and the dialog to make it clearer. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#issuecomment-898778186
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request. > @@ -3349,6 +3349,8 @@ Close Ctrl-W (C) > Closes the current fil Reload file Ctrl-R (C) Reloads the current file. +Reload allReloads all open files. Suggestion: "Reloads all open files. If changes that will be lost are detected (unsaved or saved) the reload will be confirmed, otherwise the reload will proceed without confirmation" -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#pullrequestreview-730033213
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request.
> +/* reload all files */
+void on_reload_all(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ if (!file_prefs.keep_edit_history_on_reload)
+ {
+ GeanyDocument *doc;
+ foreach_document(i)
+ {
+ doc = documents[i];
+ if (doc->changed || document_can_undo(doc) ||
document_can_redo(doc))
+ {
+ if (dialogs_show_question_full(NULL,
_("_Reload"), GTK_STOCK_CANCEL,
+ _("Any unsaved changes will be lost."),
Suggestion: "Changes detected, reloading all will lose any changes and
history."
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#pullrequestreview-730032966
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
Oh hum, didn't run into an edge case with the conditions combined but then I
didn't leave the flag on very long, diff applied. Got in the habit of letting
black format for me, miss some stuff sometimes without its help
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r688191752
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 pushed 1 commit. bf2b3176d19803b8a45f2e8d18794a26477a6ea8 Applied diff -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2859/files/f96f2d9ce14737d75aa4aa5dd040a85628681a92..bf2b3176d19803b8a45f2e8d18794a26477a6ea8
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@eht16 commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
After having a closer look and some testing, the logic and dialog response
handling is a litte wrong:
combining the conditions to show the dialog and handling its response in a
single if statement is error prone if not impossible. The current
implementation doesn't work properly.
As a suggestion, I rewrote the condition handling as follows:
```diff
diff --git a/src/callbacks.c b/src/callbacks.c
index acbd8737..487c48ac 100644
--- a/src/callbacks.c
+++ b/src/callbacks.c
@@ -340,27 +340,35 @@ void on_reload_all(GtkAction *action, gpointer user_data)
{
guint i;
gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
-
+
if (!file_prefs.keep_edit_history_on_reload)
{
+ GeanyDocument *doc;
foreach_document(i)
{
- if (!(documents[i]->changed ||
(document_can_undo(documents[i]) ||
- document_can_redo(documents[i]))) &&
dialogs_show_question_full(NULL,
- _("_Reload"), GTK_STOCK_CANCEL, _("Any unsaved changes
will be lost."),
- _("Are you sure you want to reload all files?")))
- break;
- else
- return;
+ doc = documents[i];
+ if (doc->changed || document_can_undo(doc) ||
document_can_redo(doc))
+ {
+ if (dialogs_show_question_full(NULL,
_("_Reload"), GTK_STOCK_CANCEL,
+ _("Any unsaved changes will be lost."),
+ _("Are you sure you want to reload all
files?")))
+ {
+ break; // break the loop and continue
with reloading below
+ }
+ else
+ {
+ return; // cancel reloading
+ }
+ }
}
}
-
+
foreach_document(i)
{
if (! (documents[i]->file_name == NULL))
document_reload_force(documents[i],
documents[i]->encoding);
}
-
+
gtk_notebook_set_current_page(GTK_NOTEBOOK(main_widgets.notebook),
cur_page);
}
```
This is a bit more verbose but IMO easier to read and understand.
Btw, stripping trailing spaces is always a good idea (at least for Geany code
:D).
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r688110422
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@eht16 commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
The current implementation (one dialog if any of the documents is unsaved or
can_undo/can_redo) looks good to me.
Though I didn't test it yet (will do soon). Provided it works like it should,
for me this would be fine.
@Davidy22 thanks for your patience!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r686359548
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 pushed 1 commit. f96f2d9ce14737d75aa4aa5dd040a85628681a92 Combine confirmation dialogs -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2859/files/3cd9bcd577da1904e68e9f2ef20d22e449649eba..f96f2d9ce14737d75aa4aa5dd040a85628681a92
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
Single dialog should be less annoying, I feel like this is still circumventing
the spirit of the setting a bit but I don't see why people would willingly have
the setting on anyways so I'll put it togethor sometime.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r686056953
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
> The dialog is necessary only if:
> any of the open documents has unsaved changes
> keep_edit_history_on_reload is off
Sure, a single dialog is fine, but as I noted above, in the situation that a
buffer has been changed and saved and then the file on disk is changed, for
example by git checkout, is another case where you are about to lose your
changes as the buffer is reloaded with the changed disk file. So just testing
for unsaved is not sufficient and that is why the tests in
`document_reload_prompt()` also check for any undo-able changes as an
indication that the buffer was edited and saved.
I guess that, at least in the git checkout use-case the user had to explicitly
specify files to change, since git won't overwrite modified files when just
specifying a branch or tag or commit so you don't lose them. IIUC thats the
use-case that @Davidy22 has, so I can see why he is grumpy that he gets a
prompt for each of the files that he has explicitly told git to change.
But other file-changing use-cases may not be so user friendly and we open those
users (who are not having any say in this discussion) up to the risk of data
loss if we only test for unsaved changes when keep_edit_history_on_reload is
off.
I suggest that if there is to be only one prompt then testing all files for
unsaved changes or undo/redo data should be ok, especially when it only happens
if keep_edit_history_on_reload is off.
So to summarise to ensure we are all on the same page, I am suggesting that if
keep_edit_history_on_reload is off then a loop tests buffers for unsaved
changes and undo/redo data and as soon as one is found exit the loop and make
one prompt. If the result is "reload" or keep_edit_history_on_reload was found
to be on then simply loop and reload all files with no further prompt.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r685613205
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@eht16 commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
What if we use a single confirmation dialog for the "reload all" feature which
just explains the user that Geany is going to reload all files and that any
unsaved changes will be lost. The dialog could look like "Reloading all open
documents cause any unsaved changes to be lost" [OK] [Cancel] or something like
this.
So the user has the chance to cancel the progress before anything is reloaded
and only experiences the "annoyance" of one dialog.
The dialog is necessary only if:
- any of the open documents has unsaved changes
- `keep_edit_history_on_reload` is off
I think this could work well enough for most users, assuming the majority has
`keep_edit_history_on_reload` on anyway.
(Sorry if I repeated this idea in case one of you mentioned it already earlier.)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r685552018
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
Ahh, maybe the meaning of keep_edit_history_on_reload is not entirely clear,
what it does is it also adds the _reload_ to the edit history, so its
undo-able, and all previous edits are also still there. So you don't "lose"
anything because you can undo, but if its off the edit history is cleared
because it may not apply to the newly loaded file, and now you can't undo. In
your case above, having it off means that a file with an edit history (even if
its saved) may have changes in the buffer that you are about to lose because
the file has been overwritten, and you are about to load that over the modified
buffer. So its correct to pop a prompt.
Saving the reload in undo is unusual behaviour, I'm not aware of any other
editor or IDE that does it, and for sure it can use lots of memory if you do a
big file lots, thats why the option is there. But with today's gigabyte
memories, outside unusual use-cases like watching large logs that update often,
its better to operate with keep_edit_history_on_reload setting on, which is the
default.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r685141887
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
Oh no i mean that when keep_edit_history_on_reload is off, there's a
confirmation window stopping people from reloading when there's edit history on
a tab even when the file is saved, and this pops when I use CTRL+R on a saved
document too, so conditionally circumventing it in reload all seems against the
existing flow even if it's annoying. Edge case anyways because you have to go
into a tab no one's going to visit to toggle a setting that's a bit annoying
when turned off.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r685109057
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
> Pretty sure automatic detection doesn't do it as you switch to the tab
Hmmm, on checking it seems as if it is only done if you switch tabs by the tab
right menu, not if you click the tab, I wonder why? I had the impression it
was done each time a tab switch happened to ensure the infobar came up. Seems
more like omission than intention. Will raise an issue.
> file_prefs.reload_clean_doc_on_file_change is on ... it sounds like a world
> of accidental data loss
Correct, which is why its off by default and its hidden in the various prefs.
But some people use Geany to watch logs and other things that regularly update,
and they insist they need it. So its available as caveat emptor.
> keep_edit_history_on_reload to conditionally quietly reload files with an
> edit history
The prompt should only appear if `files.keep_edit_history_on_reload` is off, if
its on then data loss is avoided because the reload is undo-able. So automatic
reload can happen irrespective of the buffer state. That seems to be what
https://github.com/geany/geany/blob/90c6096ed6ea167f9100ce8f74229a3f47acc29a/src/document.c#L1651
does, but please let us know if it seems not to work.
The various tab seems to have become a catch all for settings because making
changes to the glade file for the other preferences tabs was too hard when it
was necessary to use a Glade version that was GTK 2 compatible and that no
distro provided, so the dev had to compile it themselves. That might get
better now we no longer use GTK 2. But the reload_clean_doc_on_file_change
should stay hidden IMO.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r685086456
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
Pretty sure automatic detection doesn't do it as you switch to the tab, unless
file_prefs.reload_clean_doc_on_file_change is on and it isn't on by default and
it sounds like a world of accidental data loss anyways. Also it seems like the
various tab is the only place where you can access it, and with the way the
various tab looks no one is ever going to just browse that tab so features that
can only be enabled through it basically don't exist unless you're told that
they can be found in there. 1 refresh better than hitting the key 15 times
after you run a git checkout, and manual refresh >>> magic automatic refresh no
matter what background program silently changed your file.
I'll try adding the save check, see if this feels better, but it feels like it
goes against the spirit of the dialog that pops with
keep_edit_history_on_reload to quietly reload files with an edit history. The
whole annoyance only happens with that flag anyways, and I think anyone who has
that flag on doesn't mind being nagged a bunch.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r685036383
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
Right, that function is normally called by the response to reload on the file
change detection info bar so it only happens one file at a time as you switch
to the tab, so its less annoying. Also the info bar doesn't show and the
reload is automatic if the buffer is not changed since last save and
`file_prefs.reload_clean_doc_on_file_change` is set. So probably you could
include that test before you called `document_reload_prompt()` to fully
replicate the behaviour of the automated detection.
But it does beg the question why you need reload all since the automatic
detection will do it as you switch to the tab?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r684989460
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 pushed 1 commit. 3cd9bcd577da1904e68e9f2ef20d22e449649eba Switch to the one with the prompt -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2859/files/0160b4ca58e57d49bcb40dbf8630d86cad1ef17f..3cd9bcd577da1904e68e9f2ef20d22e449649eba
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
So I've been using it for a bit and I guess it's technically correct, it
notifies when there's undo history on a tab about to be reloaded and the
message reflects that, but it's kind of annoying when tabs with saved documents
are throwing the notification. I can see why this setting is true by default.
If this is actually what we want I'll add it to the pull request.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r684889145
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
I am not at my Geany machine ATM to check the details, but can't you just use
the function @eht16 linked to which already does it?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r684882817
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
So, just pop the window when files with changes are being refreshed with the
setting off? Alright should be doable.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r684875757
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
The problem with reload all is that even though it is user action, the user can
have so many files open that not all the tabs are visible (guilty) so the user
can forget that they have a buffer that is modified since they can't see the
tab, and then reload all will lose their work. The approach pointed out by
@eht16 is the result of much discussion and I agree his suggestion to maintain
that behaviour is reasonable.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r68473
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@eht16 commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
I guess we are both right: there is a setting (in the preferences dialog in the
"Various" tab): "files.keep_edit_history_on_reload" and if this option is not
enabled and the document has been modified, CTRL+R *will* display a
confirmation dialog (see
https://github.com/geany/geany/blob/master/src/document.c#L1651).
And so we could maintain this behaviour and either skip all modified documents
if this option is disabled or just display a similar confirmation dialog as the
current one before the reloading is performed.
While I agree with you that using the keyboard shortcut is probably a dedicated
decision for most users, we still should respect that setting. Reloading
unsaved changes while keeping the Undo history is fine but without the history,
it might affect the user.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r684728552
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 pushed 1 commit. 0160b4ca58e57d49bcb40dbf8630d86cad1ef17f Amendments as per review -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2859/files/30b104d25c7e68b0c74b2b22e033e52012fe1ab0..0160b4ca58e57d49bcb40dbf8630d86cad1ef17f
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 commented on this pull request. > @@ -244,6 +244,8 @@ gboolean document_reload_prompt(GeanyDocument *doc, const > gchar *forced_enc); void document_reload_config(GeanyDocument *doc); +void force_reload_all(void); Whoops, will clean -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#discussion_r684619595
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 commented on this pull request. > @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action, > gpointer user_data) document_reload_prompt(doc, NULL); } +/* reload all files */ +void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data) sort of was hoping there might be one but I'll rename -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#discussion_r684619702
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
But when a user hits CTRL+R to reload a single file that they've changed,
they're expecting it to reload regardless of changed status. Hitting the
keybinding being a conscious choice should mean that the user knows what they
want
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r684619540
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
The logic should probably match #1246 where issues around when to reload
automatically were thrashed out.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#discussion_r684611206
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@eht16 requested changes on this pull request.
Looks good and works as expected.
Beside my other minor comments, adding the keybinding to
https://github.com/geany/geany/blob/master/doc/geany.txt#file-keybindings would
be cool.
> @@ -244,6 +244,8 @@ gboolean document_reload_prompt(GeanyDocument *doc, const
> gchar *forced_enc);
void document_reload_config(GeanyDocument *doc);
+void force_reload_all(void);
Seems like a left-over.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
The function name somewhat suggests there is a toolbar button while it is not.
It might be unlikely that we will add a toolbar item for this functionality, so
I think it's worth to rename the function to make it less confusing.
> @@ -335,6 +335,21 @@ void on_toolbutton_reload_clicked(GtkAction *action,
> gpointer user_data)
document_reload_prompt(doc, NULL);
}
+/* reload all files */
+void on_toolbutton_reload_all_clicked(GtkAction *action, gpointer user_data)
+{
+ guint i;
+ gint cur_page =
gtk_notebook_get_current_page(GTK_NOTEBOOK(main_widgets.notebook));
+
+ foreach_document(i)
+ {
+ if (! (documents[i]->file_name == NULL))
Fine!
What do you think about additionally ignoring documents which have
`doc->changed` set?
It might be surprising for the user if changed documents get reloaded and the
unsaved changes will be lost without a warning.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2859#pullrequestreview-724802554
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request. > @@ -154,6 +154,7 @@ enum GeanyKeyBindingID GEANY_KEYS_FILE_CLOSE, /**< Keybinding. */ GEANY_KEYS_DOCUMENT_REPLACETABS,/**< Keybinding. */ GEANY_KEYS_FILE_RELOAD, /**< Keybinding. */ + GEANY_KEYS_FILE_RELOAD_ALL, /**< Keybinding. */ Correct, sorry. I did say I was short of time, I'm commenting between other things so they may contain erorrs or be incomplet. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#discussion_r681409121
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 commented on this pull request. > @@ -154,6 +154,7 @@ enum GeanyKeyBindingID GEANY_KEYS_FILE_CLOSE, /**< Keybinding. */ GEANY_KEYS_DOCUMENT_REPLACETABS,/**< Keybinding. */ GEANY_KEYS_FILE_RELOAD, /**< Keybinding. */ + GEANY_KEYS_FILE_RELOAD_ALL, /**< Keybinding. */ couldn't find apidata but a plugindata.h had a number to increment -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#discussion_r681406262
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 pushed 1 commit. 30b104d25c7e68b0c74b2b22e033e52012fe1ab0 Properly increment api version -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2859/files/a662e9b1b62b04d8011a7db970095827e833ef6a..30b104d25c7e68b0c74b2b22e033e52012fe1ab0
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 commented on this pull request. > @@ -3392,6 +3392,16 @@ static void force_close_all(void) main_status.closing_all = FALSE; } +void force_reload_all(void) Done -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#discussion_r681403560
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 pushed 1 commit. a662e9b1b62b04d8011a7db970095827e833ef6a Amendments as per review, return to current open tab after reload. -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2859/files/140e7400ea675e2273c76d8e5c9fe24aec92a47b..a662e9b1b62b04d8011a7db970095827e833ef6a
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
Just for knowledge of people watching, adding to API is fine, but do increment the count, adding to the ABI should be avoided at all costs as it blocks all older plugins from loading. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#issuecomment-891465790
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request. > @@ -154,6 +154,7 @@ enum GeanyKeyBindingID GEANY_KEYS_FILE_CLOSE, /**< Keybinding. */ GEANY_KEYS_DOCUMENT_REPLACETABS,/**< Keybinding. */ GEANY_KEYS_FILE_RELOAD, /**< Keybinding. */ + GEANY_KEYS_FILE_RELOAD_ALL, /**< Keybinding. */ Good point, you have added to the API, so yes inc the number in `apidata.h`, and "since API ..." in the comment. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#discussion_r681387246
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@Davidy22 commented on this pull request. > @@ -154,6 +154,7 @@ enum GeanyKeyBindingID GEANY_KEYS_FILE_CLOSE, /**< Keybinding. */ GEANY_KEYS_DOCUMENT_REPLACETABS,/**< Keybinding. */ GEANY_KEYS_FILE_RELOAD, /**< Keybinding. */ + GEANY_KEYS_FILE_RELOAD_ALL, /**< Keybinding. */ Oh, sorry, didn't know so I put it next to the other reload. I see an API number and version in a comment, should I add one and increment? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#discussion_r681386093
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
Couple of quick comments, don't have time for detailed check or test ATM. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#issuecomment-891462179
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request. > @@ -3392,6 +3392,16 @@ static void force_close_all(void) main_status.closing_all = FALSE; } +void force_reload_all(void) Since this is only called once suggest you put it in the callback, Geany is exhausting enough playing spaghetti functions :grin: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#pullrequestreview-720723962
Re: [Github-comments] [geany/geany] Add reload all keybinding (#2859)
@elextr commented on this pull request. > @@ -154,6 +154,7 @@ enum GeanyKeyBindingID GEANY_KEYS_FILE_CLOSE, /**< Keybinding. */ GEANY_KEYS_DOCUMENT_REPLACETABS,/**< Keybinding. */ GEANY_KEYS_FILE_RELOAD, /**< Keybinding. */ + GEANY_KEYS_FILE_RELOAD_ALL, /**< Keybinding. */ Please make additions at the end of the list or it breaks the ABI -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859#pullrequestreview-720723168
[Github-comments] [geany/geany] Add reload all keybinding (#2859)
Unbound reload all keybinding for reloading all open files, so that you don't have to individually reload every open file after an automated tool changes all of them. I wanted to make this CTRL+SHIFT+R to mirror the regular reload shortcut, but it looks like that's already taken so I made it unbound by default and bound it on my machine. I didn't make UI buttons for it, but if it's something we want UI buttons for I could do a little more later. You can view, comment on, or merge this pull request online at: https://github.com/geany/geany/pull/2859 -- Commit Summary -- * Add reload all keybinding -- File Changes -- M src/callbacks.c (6) M src/callbacks.h (2) M src/document.c (10) M src/document.h (2) M src/keybindings.c (5) M src/keybindings.h (1) -- Patch Links -- https://github.com/geany/geany/pull/2859.patch https://github.com/geany/geany/pull/2859.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/2859
