Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Splitwindow2 #185

Closed
wants to merge 19 commits into from
Closed

Splitwindow2 #185

wants to merge 19 commits into from

Conversation

kugel-
Copy link
Member

@kugel- kugel- commented Nov 3, 2013

Initial PR for multiple document notebooks. Seems to work pretty stably now.

Please ignore c73a066 (Fix geany crash on start.) for review, it's needed to fix a crash on my machine with gtk 3.10.2.

@codebrainz
Copy link
Member

I only did a very high-level pass on the diff looking for obvious bugs (got up to 15c3a52). Most of my comments can be ignored (or dealt with when finally merging). Will continue reviewing once I get some more free time. It's hard for me to review this without getting into details about the over-all architecture that could/should be fixed (but is existing and not in scope of these changes I guess), so I may not do much more than finish the high-level bug hunting pass.

@elextr
Copy link
Member

elextr commented Nov 11, 2013

As you can see below, there is a merge conflict in notebook.c with b4n's recent (void) commits. For my testing I just took your version, can you check if thats right and fix if necessary.

@b4n
Copy link
Member

b4n commented Apr 9, 2015

BTW, I see some of the discussions we had a long time ago isn't here --and may have only be on IRC.

From the top of my head, one practical issue I had with this was that when not enabled the slit window still wasted screen space for the DnD target area. And disrupting the UI when the feature is not used is unacceptable IMO.
Apart that I guess I can get over the "not the best implementation I'd wish for" problem (as I can easily admit that a perfect impl would be a lot of work rewriting many many things).

kugel- added 19 commits July 13, 2015 08:04
The purpose of this class is to encapsulate notebook tabs and their contents.
This includes management of the tab label and icon.
This allows for a clearer separation between notebook and GeanyDocument,
though GeanyPage. The notebook, being a pure GUI element, shouldn't know
about what it displays, except where it must (e.g. the tab label).

Now the notebook takes a GeanyPage which has a label, and a unspecified content
widget. notebook_new_tab() doesn't receive the corresponding GeanyDocument
anymore.
tab_count_changed() is not necessary because the same can be achieved
with the "page-added" and "page-removed" signals. This also automatically
works for other other cases of page count changes.

Also remove dead setup_tab_dnd() code. The "page-reordered" handler does nothing.
on_notebook_switch_page() has only the page widget at hand when manipulating
the mru list. And a future commit will add a similar signal handler that needs
to do the same. To save multiple costly sci->doc lookups the mru list just
stores the widgets instead. It also allows removing on_document_close()
from notebook.c (merged into removed-page handler).
As this sets up some signals for document it better fits there and notebook.c
is abstracted even more. Also, the way the current document is determind
is changed: now the current doc pointer is stored when its sci gets the focus,
as this works better with multiple notebooks.
The notebook is initialized in the same manner as the first one. main_widgets
gets a notebooks array. Its notebook member is kept (for now) to maintain ABI
compatibility.
…books

This commit largely reworks code that accesses main_widgets.notebook directly
so that it works with multiple notebooks.
When selecting a document via tab bar right click menu move it to
the notebook for which the right click menu was opened.
…ebooks

Previously it was only updated when switching tabs. This leads to the MRU
not being updated when notebooks are switched (without changing their respective
current page). This commit fixes it.

This commit also enforces that, when clicking the tabs itself on the notebooks,
that the corresponding sci widget is focused.
The switch-page handler in callbacks.c handled a lot of GUI work needed
when document tabs are changed. The same work is needed when the focus between
notebooks changes (this is the same as a tab switch but does not cause the
signal).
Now that there are multiple notebooks each one can have an active tab. The
only way to see the focussed document would be via the current-line style
of Scintilla. By underlining the tab label it should be more easy to notice
and not depend on the color theme.
Instead of the [files] key having all session files each notebook gets a
[notebookN] key which lists the documents (and the current one). The [files]
section is still restored in order to transition old config files.
This will add logic so that empty notebooks are minimized. The user-set paned
position will be remembered and restored once both notebooks contain tabs again.
The position is remembered under the "editor_notebooks_position" key in
geany.conf.
This requires installing a workaround for what appears to be a GTK bug. The
default drag-drop handler leads to assertion failures. A thin, custom one that
mimics GTK default handler doesn't have this problem but still allows to to
drop files from file managers.
The document notebooks can be split vertically and horizontally. This commits
makes the orientation user-changeable.
@kugel-
Copy link
Member Author

kugel- commented Jul 13, 2015

I'll open or re-open when I think it's ready again

@kugel- kugel- closed this Jul 13, 2015
@kugel-
Copy link
Member Author

kugel- commented Jul 13, 2015

FWIW, I did not stop working on it, quite the contrary. The code is still open in my frok (https://github.com/kugel-/geany/tree/splitwindow2)

@buserror
Copy link

I use geany all day long, and that's really a must have BTW. I tried for a while using two windows, but it's nowhere near as useful..

@kugel-
Copy link
Member Author

kugel- commented Jul 15, 2015

You can use the branch I linked above, I update it more or less regularly. But I may force push.

I just closed this PR because the current implementation is not ready for geany core so I don't want to waste the reviewers precious time.

@b4n b4n mentioned this pull request Nov 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants