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

feat(layout/#592): layout tabs - part 1 #2013

Merged
merged 13 commits into from
Jun 26, 2020

Conversation

glennsl
Copy link
Member

@glennsl glennsl commented Jun 25, 2020

layout tabs

This adds a new concept of "layout tabs", which behave more like the tabs in vim. By default these tabs are positioned at the bottom to avoid two levels of tabs at the top, but they can be positioned at the top using the oni.layout.layoutTabPosition configuration setting. They can also be hidden, or made to always show, using oni.layout.showLayoutTabs.

To mimic the layout of vim, use:

"oni.layout.layoutTabPosition": "top",
"editor.workbench.showTabs": false,

There's still a bit of work to get faithful vim behaviour though. These are the next steps I plan to work on:

  • Wire up the :tab* commands (:tabnew and :tabedit already work, although not entirely correctly)
  • Wire up gt and gT. This must be done from libvim for the moment due to bugs in editor-input, but shouldn't be too hard.
  • Add a configuration setting for a "single tab mode" that will reuse the first tab instead fo adding hidden tabs when showTabs is off.
  • For technical reasons it is currently not possible to open a new buffer directly in a new split or layout tab. Instead what will happen is that the new split or layout tab is opened with he currently active editor in addition to the new one. This is because opening a buffer will automatically open an editor (via BufferEnter), and it's not possible to open an empty split or layout tab before opening the buffer. Therefore the only two options are to either open a new split or layout tab with the currently active editor and then open the buffer, or to open the buffer in the old split/layout tab before opening a new one. Either way there'll be an extra editor that shouldn't be there.

Addresses #592

@glennsl glennsl requested a review from bryphe June 25, 2020 16:28
@bryphe
Copy link
Member

bryphe commented Jun 25, 2020

Wow, this is looking great @glennsl ! Thanks for the work on this.

@bryphe
Copy link
Member

bryphe commented Jun 25, 2020

Wire up gt and gT. This must be done from libvim for the moment due to bugs in editor-input, but shouldn't be too hard.

I can help with this piece, if you'd like, and haven't started it -

@bryphe
Copy link
Member

bryphe commented Jun 25, 2020

One thing to think about that isn't called out on your list is the quit behavior, too - if :q is used in the layout tab, it should be closed, moving to the previous tab. If it's the last layout tab, the editor should be closed.

Related to this - if I close (press the x) of the active tab, when there are tabs available, the editor closes. I would expect it to switch to the next available tab.

src/Components/Tabs.re Outdated Show resolved Hide resolved
Copy link
Member

@bryphe bryphe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just had minor feedback around the quit/close behavior - but this is awesome to finally have layouts decoupled like this. Overall looks great!

@glennsl
Copy link
Member Author

glennsl commented Jun 25, 2020

I can help with this piece, if you'd like, and haven't started it -

Already working on it. Implemented it twice even! First as an extension of the goto callback, but then after discovering it was tightly (and weirdly) tied to multiple :tab commands I decided to go with a separate tabpage callback.

One thing to think about that isn't called out on your list is the quit behavior, too - if :q is used in the layout tab, it should be closed, moving to the previous tab. If it's the last layout tab, the editor should be closed.

Related to this - if I close (press the x) of the active tab, when there are tabs available, the editor closes. I would expect it to switch to the next available tab.

Ah, yeah, sorry. That behaviour was intended only for when the last layout tab was closed. I forgot to test and handle these other cases, so it just falls back to that. Will fix!

@glennsl
Copy link
Member Author

glennsl commented Jun 25, 2020

The Ubuntu 16 CI seems to fail consistently on esy bootstrap now. I think this is the relevant part of the log:

    autoconf.texi:8017: misplaced }
    autoconf.texi:8018: must be after `@defmac' to use `@defmacx'
    autoconf.texi:8019: misplaced }
    autoconf.texi:8206: must be after `@defmac' to use `@defmacx'
    autoconf.texi:8271: misplaced }
    autoconf.texi:8290: misplaced }
    autoconf.texi:8317: misplaced }
    autoconf.texi:8380: must be after `@defmac' to use `@defmacx'
    conftest.c:4597: must be after `@defmac' to use `@defmacx'
    conftest.c:15929: must be after `@defmac' to use `@defmacx'
    Makefile:251: recipe for target 'autoconf.info' failed

Makes no sense to me though...

@bryphe
Copy link
Member

bryphe commented Jun 25, 2020

Already working on it. Implemented it twice even! First as an extension of the goto callback, but then after discovering it was tightly (and weirdly) tied to multiple :tab commands I decided to go with a separate tabpage callback.

Cool, sounds good. Thanks for looking into it!

The Ubuntu 16 CI seems to fail consistently on esy bootstrap now. I think this is the relevant part of the log:

Ouch, ya, I'm looking into it - it's unrelated to your changes. I also see this in the log:

    /home/vsts/.esy/3/b/autoconf-dafb3f25/build-aux/missing: line 51: aclocal-1.11: command not found
    WARNING: 'aclocal-1.11' is missing on your system.  You should only need it if
             you modified 'acinclude.m4' or 'configure.ac'.  You might want
             to install the Automake and Perl packages.  Grab them from
             any GNU archive site.

It shouldn't need to run aclocal at all, so it's curious why it's trying... In the meantime, if this is the only check that's failing - feel free to ignore / override. I'll work on getting that check back up.

@glennsl
Copy link
Member Author

glennsl commented Jun 25, 2020

The closing behaviour should be fixed now. But please do check thoroughly, as it's surprisingly complex with all the different levels and ways of closing.

@glennsl glennsl merged commit d9ba249 into onivim:master Jun 26, 2020
@glennsl glennsl deleted the feat/layout/layout-tabs branch June 26, 2020 08:40
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.

2 participants