Fix startup files order when placing tabs next to the current one #3611
Conversation
|
LGBI |
Looks good to me based on the code itself and I think it could go to 2.0. I just haven't tested it yet (which I'm going to do now). |
I just tested it and, yes, it is fixed, and, yes, the re-arrangement is really annoying without this patch so I think it should go to 2.0. (Somehow, I didn't have the option to place new tabs behind the current one enabled so I didn't notice the problem.) One unrelated glitch I noticed is that when you close Geany with some files opened, and then open it from the command-line with some file as an argument, this file is always opened in the second tab - I would expect it to open behind the tab which was last active when Geany closed. I expect that when the command-line file gets opened, the "current" file is still the first one so it appears behind it. But not really a big problem and maybe even not worth fixing as users hopefully forget which tab was active when they closed Geany :-). |
|
Actually if it's like this in 1.38 already (and possibly earlier) I see no compelling reason to rush this into 2.0, considering that tomorrow is release. It's not even clear to me if this is even a bug (as in "not working as intended"). I surely never used that pref. Edit: 2 approvals beat me anyway :-) |
src/keyfile.c
Outdated
| i = file_prefs.tab_order_ltr ? 0 : (session_files->len - 1); | ||
| while (TRUE) | ||
| file_prefs.tab_order_beside = FALSE; | ||
| file_prefs.tab_order_ltr = TRUE; |
There was a problem hiding this comment.
Why do you override the pref temporarily? Is there some nested function call that looks at the pref?
There was a problem hiding this comment.
Yes, notebook_new_tab(). Call chain is open_session_file() → document_open_file_full() → document_create() → notebook_new_tab().
There was a problem hiding this comment.
There was a problem hiding this comment.
@kugel- OK, makes sense. Do you like it better like that (updated)?
There was a problem hiding this comment.
yes (although I had a hard time to decode the new block in my brain)
There was a problem hiding this comment.
Nice! It:
- Fixes the issue of the reordered tabs
- Also opens a file from the command-line as the last tab which is much more logical than the second tab before
- Eliminates some ugly code
The code looks good to me as well.
There was a problem hiding this comment.
I had only a quick look on the code but tested it again and it works fine.
@b4n Hah, it seems you have proven me as crazy again, my config says |
I hardly can imagine this could be intended behaviour. After start, the previous code opened the tabs in some random (maybe not really random but in some for the user undeterministic order) and after a second start the previous order was restored, on the third start the pseudo random order of the first start is used and so on. |
Well, to me it's definitely a bug as the session is not restored as it was saved. Both documents order and current document are lost (well, current document is OK if it's the first one -- and that's likely why I didn't notice very often as I usually have the same doc I come back to a lot as the first one). And having added the feature back then, I can tell you I didn't plan for this :) |
Alright, I am convinced it's a bug now :-) |
When the `tab_order_beside` option is enabled, tabs are opened on either side of the active tab, depending on `tab_order_ltr`, rather than at either end. However, when loading startup files we don't switch to the last opened document right away, but postpone this to the very end. This affects the order tabs are loaded, as the active one is always the first one at this stage. Rather than over-complicating the loading order to compensate the various options' effect, temporarily revert to an set of options that is easy to handle, and restore it after loading is finished. This also simplifies the existing logic as a basic forward iteration is now enough. Fixes geany#3609.
|
(I just squashed the commits and rebased on master, will now merge) |
|
FFS STOP THE LAST MINUTE MERGING!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! The CI ddin't even finish here. |
|
What bad things should happen? @b4n committed (this is without any irony!). |
|
@elextr regarding the CI it was merely a squash (no diff from the base verified) + a rebase… and yes, I let my build and meson finish :P And it's all green now, ain't it :] (though, I start to think it's becoming late indeed… I'll only plan to make one more PR for 2.0 GP :]) |
|
GP is less concerning, plugins can be disabled, code in Geany can't. |
When the
tab_order_besideoption is enabled, tabs are opened on either side of the active tab, depending ontab_order_ltr, rather than at either end.However, when loading startup files we don't switch to the last opened document right away, but postpone this to the very end. This affects the order tabs are loaded, as the active one is always the first one at this stage.
Rather than over-complicating the loading order to compensate the various options' effect, temporarily revert to an set of options that is easy to handle, and restore it after loading is finished. This also simplifies the existing logic as a basic forward iteration is now enough.
Fixes #3609.