This PR is about adding features to make it possible to sort the tabs in the editor area. It includes the following changes:
04eb16b Add capability to automatically sort editor tabs 138dd09 Add keybindings for sorting editor tabs 803d6a6 Add capability to sort editor tabs a64749e Allow document_get_notebook_child() to be used globally
I chose to place them in one PR so everything can be discussed in one place. Each update depends on the other update that comes earlier than it. Which means, if the last update is wanted (`04eb16b`), all other updates that came before it should be included, but if only `803d6a6` is wanted, it would only require `a64749e`.
Here are the snapshots:
http://imgur.com/6uYVd72 http://imgur.com/G4Fi9iV http://imgur.com/VNOJaVv http://imgur.com/Pb00zIK
The first 2 updates (`803d6a6` and `138dd09`) made a fair amount of changes in the code, but the last update had to be a little aggressive since the only way to get it done properly is to alter the codes that call `document_open_file()`. Modifying `document_open_file()` itself would mean that every file opened would cause the order of tabs in the notebook widget to be recalculated and rearranged.
This could cause significant slowdown during startup time especially when opening a lot of files. That's why the only proper way to do it is to make changes on the calling functions instead, where we could only allow a function like `notebook_auto_sort_tabs()` to be called once after multiple files are opened.
We can also create a wrapper function like `document_open_file_and_auto_sort_tabs()` but it would only apply to calling functions that only open a file once everytime they are called.
It's unlikely for the last update to be merged because of that, but please also consider the first two (`803d6a6` and `138dd09`). I can create another PR for those if wanted.
You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1144
-- Commit Summary --
* Allow document_get_notebook_child() to be used globally * Add capability to sort editor tabs * Add keybindings for sorting editor tabs * Add capability to automatically sort editor tabs
-- File Changes --
M data/geany.glade (30) M src/callbacks.c (4) M src/document.c (16) M src/document.h (2) M src/keybindings.c (22) M src/keybindings.h (2) M src/keyfile.c (15) M src/libmain.c (2) M src/msgwindow.c (6) M src/notebook.c (110) M src/notebook.h (14) M src/osx.c (3) M src/prefs.c (40) M src/symbols.c (8) M src/ui_utils.c (7) M src/ui_utils.h (2) M src/win32.c (10)
-- Patch Links --
https://github.com/geany/geany/pull/1144.patch https://github.com/geany/geany/pull/1144.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/1144
@konsolebox pushed 1 commit.
e943a3d Fix syntax error on win32.c
--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1144/files/04eb16b326fcbf291c3bf71118ead...
Like the idea of sorting tabs though I think it should just be on user command, not automatically resorted on each open. When I get a sidetrack I want the files associated with that sidetrack to stay together to be easy to close, not get sorted through the files already open.
From your description that would also get rid of the need for some of your changes I suspect.
Points off for not doing the documentation, these are user visible features.
--- 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/1144#issuecomment-233951907
When I get a sidetrack I want the files associated with that sidetrack to stay together to be easy to close, not get sorted through the files already open.
But this can be enabled or disabled.
Points off for not doing the documentation, these are user visible features.
I'll consider that.
Mind if I squash the last commit about syntax error? Next time I'll find a way to test it in a win32 machine.
--- 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/1144#issuecomment-233956820
But this can be enabled or disabled.
Ok, if all automated sorting can be disabled then its fine.
I'll consider that.
The PR probably won't be accepted without.
Leave squashing until the end, there will probably be more changes in a PR this big.
--- 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/1144#issuecomment-233958794
The PR probably won't be accepted without.
Ok, I'll see what I can do.
--- 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/1144#issuecomment-233960190
@konsolebox pushed 1 commit.
764ff76 Fix and enhance sorting of editor tabs based on pathnames
--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1144/files/e943a3d7b359adcd59a6bb9196d1b...
My 2 cents:
* it should use signals rather than riddling the code with additional calls every time a document is opened. Something like [`:document-new`](http://www.geany.org/manual/reference/pluginsignals_8c.html#ade8f8c5cef19212...) and [`:document-open`](http://www.geany.org/manual/reference/pluginsignals_8c.html#aac95623e4e1fae2...) * should use [Stash](http://www.geany.org/manual/reference/stash_8h.html) for new settings! * why not a plugin? * more generically, why? I mean, what is the motivation behind sorting the tabs by file name, what problem does it solve?
--- 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/1144#issuecomment-234604865
@b4n, not sure what sorting by filename does for you, but sorting by pathname is useful to keep files from differing places together, especially if they have the same filename, eg two versions of a project.
Since only the filename is visible, selecting the right tab is error prone in this situation but would be much easier when the files from each place are grouped.
--- 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/1144#issuecomment-234682794
Isn't it a degraded version of the *Documents* sidebar then?
--- 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/1144#issuecomment-234683060
Sidebar is hidden under symbols :)
--- 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/1144#issuecomment-234685355
so the proper fix would be being able to show the *Documents* and *Symbols* sidebars at the same time?
--- 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/1144#issuecomment-234685857
But then I wouldn't have space for split windows, a very common situation when looking at two sets of files.
--- 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/1144#issuecomment-234686186
so, would be a once-only switch to the *Documents* sidebar, or a contextual popup showing the same as the *Documents* popup be better?
I'm not saying the feature here isn't useful, I'm wondering what it actually tries to solve, and whether the feature here is an appropriate solution.
--- 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/1144#issuecomment-234686442
it should use signals rather than riddling the code with additional calls every time a document is opened. Something like :document-new and :document-open
My concern was that: having the sort function called from a one-call-per-file function means that the sort function would be called everytime a file is opened, even in a batch operation when it should only be called once. But your suggestion made me examine the `document_open_file()` function again, and I got an idea after I checked the call to `g_idle_add()`. It turns out we could have a function that connects to "document-open", and use that function to connect another function to the "idle" event through `g_idle_add()`. This other function which would initiate the sorting of tabs would only be connected to the event once in every session.
I chose not to also connect to "document-new" as it would affect the function of `Next to current` or `check_tab_beside`.
should use Stash for new settings!
I just followed how other things were arranged, but I'll see how that can necessarily be done.
why not a plugin?
I'm still a beginner when it comes to hacking the UI of Geany, and I can only guess if something like this can be turned into a plugin unless I already see the working product. Directly modifying Geany first for a feature like this was easier. I also only started writing this casually. I never expected it to become complex, although it's already starting to become more simplified.
As I examined it right now, `show_tab_bar_popup_menu()` doesn't even create any hooks or signals that would allow plugins to create their own menu items. I can consider converting this to a plugin if it's necessary, but personally I'd prefer that it becomes part of Geany and not a plugin, since it directly affects the main behavior of the UI, and not just something that alters the content of the document, or adds another tab in the sidebar. If this is created as a plugin, the plugin would possibly require to be highly adaptive to changes in the UI code.
more generically, why? I mean, what is the motivation behind sorting the tabs by file name, what problem does it solve?
It doesn't exactly solve anything, but just like many other features in Geany it makes using the editor more convenient. It is convenient to have the tabs arranged properly. One thing: it makes using the shortcut keys when moving from one document to another less confusing. You get to have an idea what follows another filename even if the current tab is in the end of the screen. You know that pressing a single key or twice would make you get to other document you wanted.
Sometimes when the documents list is opened, you would expect that the one that follows a file would be the one that's next in the list, but when the tabs are not arranged like how the documents are arranged in the list, using the shortcut key would make you jump to another file instead. It's confusing to see the selected file suddenly jump from top to bottom or the other way around in the documents list.
Grouping is another thing. You would want to have x.c and x.h adjacent to each other, so that you could easily switch to them either with a mouse or with a shortcut key.
so, would be a once-only switch to the Documents sidebar, or a contextual popup showing the same as the Documents popup be better?
That wouldn't help anything about the notebook tabs which we use. There's a reason why people keep it visible and use it to traverse documents, even if the documents list is already there.
--- 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/1144#issuecomment-234739288
@konsolebox pushed 5 commits.
11bfad0 Revert "Fix syntax error on win32.c" 5bd8b52 Revert "Add capability to automatically sort editor tabs" 474c0a5 Add capability to automatically sort editor tabs (v2) defc554 Add documentation 4f219bd Use GEANY_STRING_UNTITLED as directory if file is untitled.
--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1144/files/764ff765f9309a13fe9b63c3943da...
@@ -3707,6 +3740,13 @@ Move document right Ctrl-Shift-PageDown Changes the current do Move document first Moves the current document to the first position.
Move document last Moves the current document to the last position.
+Sort tabs based on filename Sort tabs based on their filenames or base names.
+Sort tabs based on pathname Sort tabs based on their full path names.
The tabs get sorted similar to how files arearranged in the documents list when Show Paths isenabled.=============================== ========================= ==================================================
I'm not sure which one is more correct: to say "sort based on filename", or to say "sort by filename". I find the former more technically correct, even though the latter is more common.
--- 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/1144/files/474c0a5206764e61dc2115dea54e5...
@@ -222,6 +222,8 @@ gint document_compare_by_tab_order_reverse(gconstpointer a, gconstpointer b);
GeanyDocument *document_find_by_id(guint id);
+GtkWidget *document_get_notebook_child(GeanyDocument *doc);
should be inside GEANY_PRIVATE
--- 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/1144/files/4f219bdad24610e674da26cf2dc97...
@@ -273,6 +273,8 @@ enum GeanyKeyBindingID GEANY_KEYS_FORMAT_SENDTOCMD7, /**< Keybinding. */ GEANY_KEYS_FORMAT_SENDTOCMD8, /**< Keybinding. */ GEANY_KEYS_FORMAT_SENDTOCMD9, /**< Keybinding. */
- GEANY_KEYS_NOTEBOOK_SORTTABS_FILENAME, /**< Keybinding. */
- GEANY_KEYS_NOTEBOOK_SORTTABS_PATHNAME, /**< Keybinding. */
needs to go just before `GEANY_KEYS_COUNT` not to break 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/1144/files/4f219bdad24610e674da26cf2dc97...
@@ -531,6 +550,8 @@ void notebook_init(void) /* in case the switch dialog misses an event while drawing the dialog */ g_signal_connect(main_widgets.window, "key-release-event", G_CALLBACK(on_key_release_event), NULL);
- g_signal_connect(geany_object, "document-open", G_CALLBACK(on_document_open), NULL);
you probably also need to catch `document-save` in case the document has been saved as.
--- 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/1144/files/4f219bdad24610e674da26cf2dc97...
else if (interface_prefs.auto_sort_tabs_pathname)notebook_sort_tabs(NOTEBOOK_TAB_SORT_PATHNAME);- }
- on_idle_auto_sort_hooked = FALSE;
- return FALSE;
+}
+static gboolean on_document_open(GeanyDocument* doc) +{
- if (!on_idle_auto_sort_hooked && interface_prefs.show_notebook_tabs &&
(interface_prefs.auto_sort_tabs_filename || interface_prefs.auto_sort_tabs_pathname))- {
if (g_idle_add(on_idle_auto_sort_tabs, NULL) > 0)on_idle_auto_sort_hooked = TRUE;
wouldn't it be better to only sort the new document? In case someone changed sorting, it would probably not be expected that opening a new file re-sorts everything. It would also likely be cheaper, simply walking the list once finding the appropriate new position.
--- 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/1144/files/4f219bdad24610e674da26cf2dc97...
My concern was that: having the sort function called from a one-call-per-file function means that the sort function would be called everytime a file is opened, even in a batch operation when it should only be called once.
Mmmokay, but that's probably also premature optimization, it's not likely to be *that* slow.
And as said in an inline comment, you could avoid sorting everything and only sort the new document's tab.
should use Stash for new settings!
I just followed how other things were arranged, but I'll see how that can necessarily be done.
at the top of `save_dialog_prefs()` you can read `/* new settings should be added in init_pref_groups() */` ;)
As I examined it right now, `show_tab_bar_popup_menu()` doesn't even create any hooks or signals that would allow plugins to create their own menu items.
No indeed, that's not currently possible AFAIK.
I can consider converting this to a plugin if it's necessary, but personally I'd prefer that it becomes part of Geany and not a plugin, since it directly affects the main behavior of the UI, and not just something that alters the content of the document, or adds another tab in the sidebar. If this is created as a plugin, the plugin would possibly require to be highly adaptive to changes in the UI code.
Possibly fair point.
--- 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/1144#issuecomment-234741679
Slightly related is this Gist I made for someone last year who just wanted documents to remain sorted by basename: https://gist.github.com/codebrainz/3824677
--- 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/1144#issuecomment-234742888
@codebrainz plugin is small enough it looks like a candidate for the ubiquitous add-ons. Just needs by path as well.
--- 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/1144#issuecomment-234749924
@elextr yeah, though I won't work on or use the feature at all. I mostly just linked it since this Issue made me remember and lookup that gist.
--- 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/1144#issuecomment-234750540
@codebrainz no problem, left as an exercise for the reader :wink:
--- 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/1144#issuecomment-234751088
@konsolebox pushed 5 commits.
72894fe Place declaration of document_get_notebook_child() inside GEANY_PRIVATE 7e7d375 Move GEANY_KEYS_NOTEBOOK_SORTTABS_* closest to GEANY_KEYS_COUNT db8d823 Use the Stash for the auto-sort settings d8c4f70 Fix wrong definition of callback function 646e815 Also auto-sort when document is saved to a new file
--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1144/files/4f219bdad24610e674da26cf2dc97...
else if (interface_prefs.auto_sort_tabs_pathname)notebook_sort_tabs(NOTEBOOK_TAB_SORT_PATHNAME);- }
- on_idle_auto_sort_hooked = FALSE;
- return FALSE;
+}
+static gboolean on_document_open(GeanyDocument* doc) +{
- if (!on_idle_auto_sort_hooked && interface_prefs.show_notebook_tabs &&
(interface_prefs.auto_sort_tabs_filename || interface_prefs.auto_sort_tabs_pathname))- {
if (g_idle_add(on_idle_auto_sort_tabs, NULL) > 0)on_idle_auto_sort_hooked = TRUE;
Gradual sorting indeed is the better way. I already have a code for it but it still doesn't work as expected. I'll figure it out soon.
--- 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/1144/files/4f219bdad24610e674da26cf2dc97...
@konsolebox pushed 1 commit.
dc07e0c Simplify stuff, and make automatic sorting gradual
--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1144/files/646e815c9c7f26239a396882d5c4e...
Slightly related is this Gist I made for someone last year who just wanted documents to remain sorted by basename: https://gist.github.com/codebrainz/3824677
I've been wondering how to use a custom struct to hold document or tab info, and also how to use `g_slice_alloc()`. Your example demonstrates it well. I thought about using a custom list before, but the solution I was working on turned out be not the proper one. Still, I could use your code as a reference someday. I'll also refer to it if I convert this feature to a plugin.
--- 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/1144#issuecomment-235597792
@konsolebox pushed 1 commit.
85d5b31 Rename npages to n_pages
--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1144/files/dc07e0c0c2bcf31bd106eb37f72d5...
@konsolebox pushed 1 commit.
c064984 Rename gradually_sort_doc_* to gradually_sort_tab_*
--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1144/files/85d5b31836dc10cc3edcb051a61be...
@b4n, @elextr
Any thoughts on this? Should we convert it to a plugin? If we decide to convert it to a plugin, we can move the sort commands to the Tools menu.
And is saying "sort based on ..." good? Or should I simply just say "sort by ..."?
--- 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/1144#issuecomment-236339701
@konsolebox IMHO compared to the prototype plugin by @codebrainz this is pretty big and intrusive, so I would suggest looking at the plugin instead, you can leave this PR here in the meantime so we can compare them.
As for "sort based on" and "sort by", "sort by" is a very common expression in programming, so just use that.
--- 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/1144#issuecomment-236339989
IMHO compared to the prototype plugin by @codebrainz this is pretty big and intrusive
It's natural since this has features that it needs to keep up with: sort by pathname, manual sorting, and gradual automatic sorting. Sorting by pathname itself is a large update since it has to adapt with Document List's way of sorting things, or else the resulting arrangement would be difficult to the user.
so I would suggest looking at the plugin instead
Yes I don't mind. I actually just want this to get done.
you can leave this PR here in the meantime so we can compare them.
No problem.
As for "sort based on" and "sort by", "sort by" is a very common expression in programming, so just use that.
Thanks. I was also wanting to simplify it 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/1144#issuecomment-236342846
Of course your version is going to be bigger than the prototype plugin, but the key word was "intrusive", if a plugin has a bug the user can just disable it, but a change in core can't be disabled, so it has to be more carefully evaluated and tested. @b4n is much more cautious about core changes and the more intrusive or large they are, then the slower their acceptance.
You were wanting to move fast, and for speed of release a plugin is usually better.
--- 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/1144#issuecomment-236346959
@@ -51,6 +51,8 @@ typedef struct GeanyInterfacePrefs gchar *tagbar_font; /**< symbol sidebar font */ gchar *msgwin_font; /**< message window font */ gboolean show_notebook_tabs; /**< whether editor tabs are visible */
- gboolean auto_sort_tabs_filename; /**< whether to automatically sort tabs based on filename */
- gboolean auto_sort_tabs_pathname; /**< whether to automatically sort tabs based on pathname */
need to be added to the end of the structure not to break ABI, as the other fields are part of the API. Also, those new fields should probably not be added to the API (`/**` comment), so make the comment use `/*` only.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
@@ -754,6 +779,314 @@ void notebook_remove_page(gint page_num) }
+/* Copied from sidebar.c. We can place this in one place like utils.c. */ +static gboolean utils_filename_has_prefix(const gchar *str, const gchar *prefix) +{
- gchar *head = g_strndup(str, strlen(prefix));
- gboolean ret = utils_filenamecmp(head, prefix) == 0;
- g_free(head);
- return ret;
+}
+/* Copied from sidebar.c. We can place this in one place like utils.c,
- with a more formal naming convention. */
+static gchar *get_doc_folder(const gchar *path)
why use this so convoluted way of sorting? what would be wrong with sorting just by full path? In the sidebar we want to avoid showing a long path is we have something else and meaningful to show, but here it's not shown but only for sorting. Is it that important that the order be the very same as the sidebar -- even project-relative?
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
@@ -777,6 +781,8 @@ static void load_dialog_prefs(GKeyFile *config) file_prefs.tab_order_ltr = utils_get_setting_boolean(config, PACKAGE, "tab_order_ltr", TRUE); file_prefs.tab_order_beside = utils_get_setting_boolean(config, PACKAGE, "tab_order_beside", FALSE); interface_prefs.show_notebook_tabs = utils_get_setting_boolean(config, PACKAGE, "show_notebook_tabs", TRUE);
- if (interface_prefs.auto_sort_tabs_filename && interface_prefs.auto_sort_tabs_pathname)
interface_prefs.auto_sort_tabs_filename = FALSE;
is this useful? also, it should probably go some place else, so that it's executed when loading settings directly rather than when displaying them, shouldn't 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/1144/files/c064984ff29744294b7c25d2987c5...
Take a look at b1528f01e240379c0f75ae0c2f50829b271c0f30. AFAICT (and IMO) it doesn't make the behavior worse, and is a lot simpler. It alters several things to make them simpler, not only getting the document's base name.
And if really it needs to be the exact same as the sidebar, the code should indeed be shared rather than copied.
--- 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/1144#issuecomment-236371970
</child><child><object class="GtkCheckButton" id="check_auto_sort_tabs_pathname"><property name="label" translatable="yes">Automatically sort tabs based on pathname</property><property name="visible">True</property><property name="can_focus">True</property><property name="receives_default">False</property><property name="use_underline">True</property><property name="draw_indicator">True</property></object><packing><property name="expand">False</property><property name="fill">False</property><property name="position">6</property></packing></child>
It would probably be better UI to have a checkbox to enable sorting, and radio items or a dropdown do choose the sort method.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
</child><child><object class="GtkCheckButton" id="check_auto_sort_tabs_pathname"><property name="label" translatable="yes">Automatically sort tabs based on pathname</property><property name="visible">True</property><property name="can_focus">True</property><property name="receives_default">False</property><property name="use_underline">True</property><property name="draw_indicator">True</property></object><packing><property name="expand">False</property><property name="fill">False</property><property name="position">6</property></packing></child>
…or have one of the options to be no sorting at all.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
@@ -51,6 +51,8 @@ typedef struct GeanyInterfacePrefs gchar *tagbar_font; /**< symbol sidebar font */ gchar *msgwin_font; /**< message window font */ gboolean show_notebook_tabs; /**< whether editor tabs are visible */
- gboolean auto_sort_tabs_filename; /**< whether to automatically sort tabs based on filename */
- gboolean auto_sort_tabs_pathname; /**< whether to automatically sort tabs based on pathname */
and it would be better to have one single setting that selects the mode (`NONE`/`BASENAME`/`PATH`) instead of two mutually exclusive booleans.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
g_free(base_b);}g_free(dirname_b);++pos;- }
- g_free(base_a);
- g_free(dirname_a);
- move_tab(doc, -1);
+}
+static void on_document_open(GObject *obj, GeanyDocument *doc) +{
- if (interface_prefs.show_notebook_tabs)
this most likely is a bad case of premature optimization. Because it the user had the tabs hidden but sorting enabled, and shows the tabs, it won't be sorted as he probably would have expected.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
Of course your version is going to be bigger than the prototype plugin, but the key word was "intrusive", if a plugin has a bug the user can just disable it, but a change in core can't be disabled, so it has to be more carefully evaluated and tested.
Yeah. And generally, if a plugin fits well and doesn't require endless hacks, I generally like it that way. for non-core editing features. Not saying nothing should be in core, but that some things should be plugins unless there's a reason why it can't.
--- 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/1144#issuecomment-236372489
@@ -65,6 +66,8 @@ static gboolean switch_in_progress = FALSE; static GtkWidget *switch_dialog = NULL; static GtkWidget *switch_dialog_label = NULL;
+static gboolean doc_saves_to_new_file = FALSE;
meh, globals are evil :( even a static variable passed through as user data to the callbacks would be less ugly :]
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
@@ -51,6 +51,8 @@ typedef struct GeanyInterfacePrefs gchar *tagbar_font; /**< symbol sidebar font */ gchar *msgwin_font; /**< message window font */ gboolean show_notebook_tabs; /**< whether editor tabs are visible */
- gboolean auto_sort_tabs_filename; /**< whether to automatically sort tabs based on filename */
- gboolean auto_sort_tabs_pathname; /**< whether to automatically sort tabs based on pathname */
and it would be better to have one single setting that selects the mode (NONE/BASENAME/PATH) instead of two mutually exclusive booleans.
Agree.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
@@ -754,6 +779,314 @@ void notebook_remove_page(gint page_num) }
+/* Copied from sidebar.c. We can place this in one place like utils.c. */ +static gboolean utils_filename_has_prefix(const gchar *str, const gchar *prefix) +{
- gchar *head = g_strndup(str, strlen(prefix));
- gboolean ret = utils_filenamecmp(head, prefix) == 0;
- g_free(head);
- return ret;
+}
+/* Copied from sidebar.c. We can place this in one place like utils.c,
- with a more formal naming convention. */
+static gchar *get_doc_folder(const gchar *path)
I won't really use the word important there. But I simply find it convenient that the order would be the same, since traversing the documents using shortcut keys would be less confusing, whenever I'm looking at the sidebar.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
@@ -777,6 +781,8 @@ static void load_dialog_prefs(GKeyFile *config) file_prefs.tab_order_ltr = utils_get_setting_boolean(config, PACKAGE, "tab_order_ltr", TRUE); file_prefs.tab_order_beside = utils_get_setting_boolean(config, PACKAGE, "tab_order_beside", FALSE); interface_prefs.show_notebook_tabs = utils_get_setting_boolean(config, PACKAGE, "show_notebook_tabs", TRUE);
- if (interface_prefs.auto_sort_tabs_filename && interface_prefs.auto_sort_tabs_pathname)
interface_prefs.auto_sort_tabs_filename = FALSE;
I guess. It also probably would no longer be needed if we already get to use just one variable.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
And if really it needs to be the exact same as the sidebar, the code should indeed be shared rather than copied.
I actually just copied it to avoid more intrusion. I'm just waiting for a go signal so I could place it somewhere else like `utils.c`. How should we share the code? As what I've suggested, we can place it in `utils.h`. Or should we just place it in `sidebar.c` and have `notebook.c` include `sidebar.h`?
Also, should we have add a prefix to `get_doc_folder`? Like `utils_get_doc_folder` or `sidebar_get_doc_folder`?
--- 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/1144#issuecomment-236382767
</child><child><object class="GtkCheckButton" id="check_auto_sort_tabs_pathname"><property name="label" translatable="yes">Automatically sort tabs based on pathname</property><property name="visible">True</property><property name="can_focus">True</property><property name="receives_default">False</property><property name="use_underline">True</property><property name="draw_indicator">True</property></object><packing><property name="expand">False</property><property name="fill">False</property><property name="position">6</property></packing></child>
...or have one of the options to be no sorting at all.
I find this better.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
g_free(base_b);}g_free(dirname_b);++pos;- }
- g_free(base_a);
- g_free(dirname_a);
- move_tab(doc, -1);
+}
+static void on_document_open(GObject *obj, GeanyDocument *doc) +{
- if (interface_prefs.show_notebook_tabs)
I guess. The reason I disabled it was because shortcut keys don't work when editors tabs are disabled anyway. I'll remove 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/1144/files/c064984ff29744294b7c25d2987c5...
@@ -65,6 +66,8 @@ static gboolean switch_in_progress = FALSE; static GtkWidget *switch_dialog = NULL; static GtkWidget *switch_dialog_label = NULL;
+static gboolean doc_saves_to_new_file = FALSE;
Do you mean to have something like `g_signal_connect(geany_object, "document-open", G_CALLBACK(on_document_open), &doc_saves_to_new_file);`? Should we really do that?
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
g_free(base_b);}g_free(dirname_b);++pos;- }
- g_free(base_a);
- g_free(dirname_a);
- move_tab(doc, -1);
+}
+static void on_document_open(GObject *obj, GeanyDocument *doc) +{
- if (interface_prefs.show_notebook_tabs)
Is that so? I don't see no reason why it would be disabled?
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
@@ -65,6 +66,8 @@ static gboolean switch_in_progress = FALSE; static GtkWidget *switch_dialog = NULL; static GtkWidget *switch_dialog_label = NULL;
+static gboolean doc_saves_to_new_file = FALSE;
Do you mean to have something like `g_signal_connect(geany_object, "document-open", G_CALLBACK(on_document_open), &doc_saves_to_new_file);`?
yes
Should we really do that?
I don't know. Ideally there wouldn't be any need for a hack to know whether it was saved or saved as… BTW your technique here doesn't really work: it only finds out whether it's the first save, not whether the filename changed.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
</child><child><object class="GtkCheckButton" id="check_auto_sort_tabs_pathname"><property name="label" translatable="yes">Automatically sort tabs based on pathname</property><property name="visible">True</property><property name="can_focus">True</property><property name="receives_default">False</property><property name="use_underline">True</property><property name="draw_indicator">True</property></object><packing><property name="expand">False</property><property name="fill">False</property><property name="position">6</property></packing></child>
what do you refer to with "this"?
IMO the current UI is bad, not only because there is no visual link between the two options, but also because AFAIK it's the first use of this UI in all of Geany.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
@@ -777,6 +781,8 @@ static void load_dialog_prefs(GKeyFile *config) file_prefs.tab_order_ltr = utils_get_setting_boolean(config, PACKAGE, "tab_order_ltr", TRUE); file_prefs.tab_order_beside = utils_get_setting_boolean(config, PACKAGE, "tab_order_beside", FALSE); interface_prefs.show_notebook_tabs = utils_get_setting_boolean(config, PACKAGE, "show_notebook_tabs", TRUE);
- if (interface_prefs.auto_sort_tabs_filename && interface_prefs.auto_sort_tabs_pathname)
interface_prefs.auto_sort_tabs_filename = FALSE;
indeed
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
@@ -754,6 +779,314 @@ void notebook_remove_page(gint page_num) }
+/* Copied from sidebar.c. We can place this in one place like utils.c. */ +static gboolean utils_filename_has_prefix(const gchar *str, const gchar *prefix) +{
- gchar *head = g_strndup(str, strlen(prefix));
- gboolean ret = utils_filenamecmp(head, prefix) == 0;
- g_free(head);
- return ret;
+}
+/* Copied from sidebar.c. We can place this in one place like utils.c,
- with a more formal naming convention. */
+static gchar *get_doc_folder(const gchar *path)
It will already be totally different if you use "sort by basename"; and I find it kinda odd to want to see the sidebar *and* have the tabs sorted (which indeed would be consistent, but would be totally redundant).
I don't have a strong opinion here, I just found it a kinda artificial requirement that made the code uglier than needed.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
I actually just copied it to avoid more intrusion. I'm just waiting for a go signal so I could place it somewhere else like `utils.c`. How should we share the code? As what I've suggested, we can place it in `utils.h`. Or should we just place it in `sidebar.c` and have `notebook.c` include `sidebar.h`?
I guess keeping it in sidebar would make more sense: it's not really anything generic, it's highly specific to how the sidebar works. In that sense what you would really want to share is the sorting order, but you can't too easily do that -- apart maybe by being able to get the document list as sorted in the sidebar, and apply that very order.
Also, should we add a prefix to `get_doc_folder`? Like `utils_get_doc_folder` or `sidebar_get_doc_folder`?
yes, all functions added to headers -- event internally -- need to be prefixed.
--- 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/1144#issuecomment-236389676
@elextr pointed out to me I was not clear at all: I still prefer it to be a plugin if possible. I made my comments and review in the hope it would improve the code, but that this code would hopefully end up in a plugin.
--- 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/1144#issuecomment-236399355
I was bored last night and made such a plugin:
https://github.com/codebrainz/geany-plugins/tree/new-plugin/tabsort
@konsolebox if you would like to use it as a starting point and maintain it in Geany-Plugins, feel free, I probably won't do it myself. It probably has some bugs but seems to work OK.
--- 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/1144#issuecomment-236399692
</child><child><object class="GtkCheckButton" id="check_auto_sort_tabs_pathname"><property name="label" translatable="yes">Automatically sort tabs based on pathname</property><property name="visible">True</property><property name="can_focus">True</property><property name="receives_default">False</property><property name="use_underline">True</property><property name="draw_indicator">True</property></object><packing><property name="expand">False</property><property name="fill">False</property><property name="position">6</property></packing></child>
I find what you said better: having something like `Placement of new file tabs: ( ) Left (*) Right` but with 3 options: Disabled, By Filename, By Pathname
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
@@ -65,6 +66,8 @@ static gboolean switch_in_progress = FALSE; static GtkWidget *switch_dialog = NULL; static GtkWidget *switch_dialog_label = NULL;
+static gboolean doc_saves_to_new_file = FALSE;
Ideally there wouldn't be any need for a hack to know whether it was saved or saved as
Would that unexpectedly move a manually rearranged tab when simply just saving a file?
it only finds out whether it's the first save, not whether the filename changed.
As I observed, `doc->real_path` is always `NULL` in a `"document-before-save"` event when saving to a new file. Is there a case where this is different? I'll check it again.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
@codebrainz
@konsolebox if you would like to use it as a starting point and maintain it in Geany-Plugins, feel free, I probably won't do it myself. It probably has some bugs but seems to work OK.
I'll just finish what has to be done in this PR, then I'll see what I can do with a plugin. But your code looks robust enough. It also notice some other features like reverse sorting. It might already be the appropriate solution. Many methods are still unfamiliar to me though. I think it would be nice if you decide to maintain it instead, since you're more adept to the code, and you have more experience.
I actually already started creating a plugin earlier from scratch, as per @elextr's suggestion, and I intend to continue doing so so I get to have my own version that's closer to home. It still has a long way to go though.
At least that way we could compare different solutions.
If you decide to send a PR, I'll try to join the discussion if I notice some bugs, or if I could give some ideas about how it could be further enhanced.
--- 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/1144#issuecomment-236414255
I think it would be nice if you decide to maintain it instead, since you're more adept to the code, and you have more experience.
I don't really want to maintain this plugin as I won't dog-food it, and I already (fail to) maintain enough plugins. I just wrote it since I was bored and if you've never written it can't be hard to get everything put together and running, so I figured it would be helpful.
--- 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/1144#issuecomment-236416588
if you've never written it can't be hard to get everything put together and running, so I figured it would be helpful.
s/can't/can/ I think?
--- 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/1144#issuecomment-236417236
@konsolebox pushed 7 commits.
ed809af Say "sort by", not "sort based on" 6d99001 Share get_doc_folder as sidebar_get_doc_folder and use it. bf3d1df Use GPtrArray instead of GArray bb54fc1 Apply function simplifications by @b4n b7ba772 Do not skip automatic sorting even when editor tabs are disabled a6b84e2 Use radio buttons for the auto-sort options, and use a single parameter for it aaeab39 Convert an if-else block to a simple one-line
--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1144/files/c064984ff29744294b7c25d2987c5...
g_free(base_b);}g_free(dirname_b);++pos;- }
- g_free(base_a);
- g_free(dirname_a);
- move_tab(doc, -1);
+}
+static void on_document_open(GObject *obj, GeanyDocument *doc) +{
- if (interface_prefs.show_notebook_tabs)
The default-permanent shortcut keys still work (Page-Up and Page-Down) when Show Editor Tabs is disabled, but secondary custom keys (mine is Ctrl-Tab and Ctrl-Shift-Tab) won't.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
@@ -65,6 +66,8 @@ static gboolean switch_in_progress = FALSE; static GtkWidget *switch_dialog = NULL; static GtkWidget *switch_dialog_label = NULL;
+static gboolean doc_saves_to_new_file = FALSE;
It seems to work as expected.
--- 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/1144/files/c064984ff29744294b7c25d2987c5...
@b4n I complied with the required updates for this PR. Tell me if there's something I missed, or something still has to be changed (like if I shouldn't use another `struct` for the auto-sort option.)
Should I just add another item like `NOTEBOOK_TAB_SORT_DISABLED` or `NOTEBOOK_TAB_SORT_NONE` to `NotebookTabSortMethod`, instead of having `NotebookTabAutoSortMode`?
--- 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/1144#issuecomment-236510735
I just created another version of this. This time it has one more sorting method: Sorting by folder (which refers to the folder in the documents list in the sidebar, and does exactly what it has to do: sort documents the way the sidebar sorts them). I thought this would solve the ambiguity that makes us wonder how 'Sort by pathname' should really sort the documents: whether to copy sidebar's way of sorting them, or whether to just sort them based on their complete pathnames. This time, 'Sort by pathname' does it simpler: get the directory part through `doc->realpath`. If `realpath` is `NULL`, the directory is simply an empty string.
I placed the the changes in another branch: https://github.com/konsolebox/geany/tree/sort_tabs_v2
Aside from adding the new feature, these changes were also made: (List copied from https://github.com/konsolebox/geany/commit/7adbc1e33f983ec32bfe05006930bcbbb...)
- Sorting by pathname now relies on doc->realpath when getting the directory part. - Popup menu now only has a single sort command item (a submenu with three subitems). - No longer uses NotebookTabAutoSortMethod. Just added NOTEBOOK_TAB_SORT_NONE to NotebookTabSortMethod. Along with get_compare_method(), it made stuff much simpler. - No longer uses variables 'doc_a' and 'doc_b' since they're only used once. '*(GeanyDocument **) a|b' is just directly passed as an argument instead. It makes the code look less heavier and it saves a few lines. - Declares GeanyDocument as constant in function declarations. - 'on_sort_tabs_by_*_activate' were moved from notebook.h to notebook.c. - Documentation was updated.
And these are the preview images: http://imgur.com/a/PySmh
The last image is about the newer update (https://github.com/konsolebox/geany/commit/474c5ef027d714e9c95c8e0c0b55fdc7d...) where the radio options are converted to a combobox.
I can forward the changes here if it's wanted.
--- 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/1144#issuecomment-236888774
I'm closing this since I don't think there's interest for it to be merged to the core. I also didn't have enough enthusiasm to convert it to a plugin after it worked so well. Feel free to use the patches as reference.
Closed #1144.
github-comments@lists.geany.org