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

Make tabs detachable + minor bugfix #494

Merged
merged 4 commits into from
Aug 29, 2021

Conversation

Vulcalien
Copy link
Member

@Vulcalien Vulcalien commented Aug 25, 2021

Fixes #302

Note - This pull request is to show the progress. I have done very little testing, so I don't recommend pulling it right now.

About

As said in the issue comments, set_tab_detachable does the job. The code found on StackOverflow gave me a guideline, but much of that stuff is already implemented somewhere in Terminator. So I had to create only one function, create_window_detach.

About self.hoover: without calling that function, if a tab is detached and a notebook only has 2 tabs, the resulting notebook only has 1 tab, causing some troubles.

Most of the work was about reading the documentation.


I'll finish testing soon, and maybe fix that Drag-and-drop problem that we were talking about in the comments, since it's related.

@Vulcalien Vulcalien changed the title Make tabs detachable (fixes #302) Make tabs detachable Aug 25, 2021
@mattrose
Copy link
Member

Most of the work was about reading the documentation.

LOL, yep. There's a reason https://lazka.github.io/pgi-docs/ is one of my most-used bookmarks :)

Thanks, I'm curious to see how this is going. I'll likely pull it down and play with it tomorrow morning.

@Vulcalien
Copy link
Member Author

One bug found:

(terminator:92639): Gtk-CRITICAL **: 00:08:00.686: gtk_notebook_get_tab_label: assertion 'list != NULL' failed
Notebook::update_tab_label_text: <terminal.Terminal object at 0x7f436d33f3c0 (terminatorlib+terminal+Terminal at 0x1be29e0)> not found

I ran terminator without -d

Strange stuff. Only happens when you detach a tab from a notebook that contains 3 or more tabs. It doesn't seem to have any effect, other than the error message.

Probably I am missing a function call or something.

@Vulcalien
Copy link
Member Author

Vulcalien commented Aug 26, 2021

Probably I am missing a function call or something.

Yep, self.disconnect_child(widget). It's used in Notebook.remove after a notebook page is removed, and it makes sense to call it after a tab is detached.


Edit - there is still this:

<terminal.Terminal object at 0x7f854feefe00 (terminatorlib+terminal+Terminal at 0x16f41a0)> not found in Notebook. Actual parent is: None

To reproduce:

  1. create 3 or more tabs
  2. detach one of them
  3. close the new window
  4. close the old window from the X button

@mattrose
Copy link
Member

This looks good so far, though I agree definitely not ready for prime time yet.
One debug trick I recommend, you can specify what classes and methods for which you want debug messages to show up, so in this case I would use something like terminator -d --debug-class=Notebook or something like terminator -d --debug-class=Notebook --debug-method=create_window_detach and then you can add dbg messages to figure out what the code is doing better.

@Vulcalien
Copy link
Member Author

Updates

That strange log error is not caused by the detached tabs. I could reproduce it by simply having 3 tabs and then closing the Terminator window (using the GUI "X").
It also happens on older versions (2.1.0).

I've written a dbg message for the new function and done some testing. I think the PR could be ready.

@mattrose
Copy link
Member

So, I was playing with this last night, and this error, I believe is caused by (of all things)

label.connect('close-clicked', self.closetab)

Which means that clicking anywhere on the label of the tab (basically anywhere in the tab), it can consider that calling closetab. I'd really like to make it so that you have to click on the X button to close the tab.

@Vulcalien
Copy link
Member Author

Vulcalien commented Aug 27, 2021

Which means that clicking anywhere on the label

Wait, if you click the notebook label, the tab closes? I do have to click the X in the tab label to close it.


Btw I think I found something related. Window::on_destroy_event, these lines

for terminal in self.get_visible_terminals():
    terminal.close()

If I remove them, I see one more error message (... not found in Notebook. Actual parent is: None...) having the same number of tabs. I think all terminals should be closed, but are not since they are not visible (self.get_visible_terminals() does not return them)


Writing this after 10 minutes.
I fixed it! Instead of calling get_visible_terminals (that would close only the ones present in the current tab), close all terminals in the window, whenever they are visible or not (because they are in another notebook tab).

for terminal in self.get_terminals():
    terminal.close()

I'm including this commit in the PR.

@Vulcalien Vulcalien changed the title Make tabs detachable Make tabs detachable + minor bugfix Aug 27, 2021
@mattrose
Copy link
Member

Is this ready to go?

@Vulcalien
Copy link
Member Author

Yep, I think it is.

@mattrose
Copy link
Member

Cool, I'll pull it down again and give it a quick once-over before merging it.

@mattrose
Copy link
Member

This looks great. Thanks again!

@mattrose mattrose merged commit a7fcf27 into gnome-terminator:master Aug 29, 2021
@Vulcalien Vulcalien deleted the detachable_tabs branch August 29, 2021 22:34
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.

Feature Request: detach tab
2 participants