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

Add tabs to layouts #625

Merged
merged 35 commits into from
Aug 26, 2021
Merged

Add tabs to layouts #625

merged 35 commits into from
Aug 26, 2021

Conversation

a-kenji
Copy link
Contributor

@a-kenji a-kenji commented Jul 23, 2021

fixes #603, fixes #349

  • The layout has now a unique tabs section,
    that can be used, like the parts section,
    everything that is not inside the tabs section
    is assumed to be present on every single tab
    that is opened.

    This is a BREAKING CHANGE for people that use
    custom layouts already, since the tabs section
    is not optional - for clarity and intentionality reasons.

    The functionality to specify multiple tabs is already there,
    but is still gated behind a panic, until Messed up layout after opening tabs in close succession #621 is fixed.
    So for now one tab can be specified to load on startup.

  • The NewTab action can optionally be bound to open
    a layout that is assumed to be in the new tabs section

    This is a BREAKING CHANGE for people that have the
    NewTab action already bound in the config file:

- action: [NewTab, ]
  key: [F: 5,]

must now be specified as:

- action: [NewTab: ,]
  key: [F: 5,]

Optionally a layout that should be opened on the new tab can be
specified:

- action: [NewTab: {
  direction: Vertical,
  parts: [ {direction: Horizontal, split_size: {Percent: 50}}, {direction: Horizontal, run: {command: {cmd: "htop"}}},],
  key: [F: 6,]

or:

- action: [NewTab: {direction: Vertical, run: {command: {cmd: "htop"} }},]
  key: [F: 7,]

or

- action: [NewTab: {
  direction: Vertical,
  parts: [ {direction: Vertical, split_size: {Percent: 25},run: {plugin: "strider" }}, {direction: Horizontal}],}, MoveFocus: Left,]
  key: [F: 8,]

a-kenji added 4 commits July 23, 2021 17:13
fixes zellij-org#603, fixes zellij-org#349

* The layout has now a unique `tabs` section,
  that can be used, like the `parts` section,
  everything that is not inside the tabs section
  is assumed to be present on every single tab
  that is opened.

  This is a BREAKING CHANGE for people that use
  custom `layouts` already, since the `tabs` section
  is not optional - for clarity and intentionality reasons.

  The functionality to specify multiple tabs is already there,
  but is still gated behind a panic, until zellij-org#621 is fixed.
  So for now one tab can be specified to load on startup.

* The `NewTab` action can optionally be bound to open
  a layout that is assumed to be in the new `tabs` section

  This is a BREAKING CHANGE for people that have the
  `NewTab` action already bound in the config file:

```
- action: [NewTab, ]
  key: [F: 5,]
```
must now be specified as:
```
- action: [NewTab: ,]
  key: [F: 5,]
```
  Optionally a layout that should be opened on the new tab can be
  specified:
```
- action: [NewTab: {
  direction: Vertical,
  parts: [ {direction: Horizontal, split_size: {Percent: 50}}, {direction: Horizontal, run: {command: {cmd: "htop"}}},],
  key: [F: 6,]
```
or:
```
- action: [NewTab: {direction: Vertical, run: {command: {cmd: "htop"} }},]
  key: [F: 7,]
```
or
```
- action: [NewTab: {
  direction: Vertical,
  parts: [ {direction: Vertical, split_size: {Percent: 25},run: {plugin: "strider" }}, {direction: Horizontal}],}, MoveFocus: Left,]
  key: [F: 8,]

```
@a-kenji a-kenji requested a review from imsnif July 23, 2021 15:58
@imsnif
Copy link
Member

imsnif commented Jul 26, 2021

As discussed offline, I feel this is currently blocked by #621
Let's fix that one first so people will be able to have a layout with more than 1 tab.

@a-kenji
Copy link
Contributor Author

a-kenji commented Jul 27, 2021

Thanks for checking it!

@a-kenji
Copy link
Contributor Author

a-kenji commented Aug 1, 2021

@imsnif
#621 Is now solved.
I am now still doing some very slight modifications,
but it is ready for a review now if you'd like!

* Adjust and add tests for the change
@a-kenji
Copy link
Contributor Author

a-kenji commented Aug 2, 2021

@TheLostLambda
I added you to the reviewers here, since you are very deep in the layout system atm!

sagittarius-a and others added 12 commits August 3, 2021 21:11
Fixes zellij-org#398.

Tab key is used as default for the `GoToLastTab` action.
* work

* resize working

* move focus working

* close pane working

* selection and fullscreen working

* pane title line

* titles and conditional scroll title

* whole tab resize working

* plugin frames working

* plugin splitting working

* truncate pane frame titles

* cleanup

* panes always draw their own borders - also fix gap

* toggle pane frames

* move toggle to screen and fix some bugs

* fix plugin frame toggle

* fix terminal window resize

* fix scrolling and fullscreen bugs

* unit tests passing

* e2e tests passing and new test for new frames added

* refactor: TerminalPane and PluginPane

* refactor: Tab

* refactor: moar Tab

* refactor: Boundaries

* only render and calculate boundaries when there are no pane frames

* refactor: Layout

* fix(grid): properly resize when coming back from alternative viewport

* style: remove commented code

* style: fmt

* style: fmt

* style: fmt + clippy

* docs(changelog): update change
…g#646)

* fix(compatibility): support changing index colors with osc

* style(fmt): make rustfmt happy

* style(fmt): make clippy happy

* style(fmt): make rustfmt happy

* docs(changelog): document fix
…j-org#648)

* fix unexpected pane closing issue with nushell

nushell doesn't create a new process group when spawnning a process,
so all processes including the ones spwanned by us are in the same
foreground group. So if kernel will send signal to every member of this
group.

This patch fixes this issue by creating a new foreground process group
when spawnning a new terminal pane.

Fix zellij-org#615

Signed-off-by: Tw <tw19881113@gmail.com>

* add comment about unsafe

Co-authored-by: Aram Drevekenin <aram@poor.dev>
Copy link
Member

@TheLostLambda TheLostLambda left a comment

Choose a reason for hiding this comment

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

Heya! Overall this is looking great and I'm happy to have it merged whenever (it should only minorly conflict with what I've been doing to layouts).

Some feedback:

  1. Personally, I find the naming of TabLayouts a little confusing. Intuitively, I'd think that the TabLayout is the bigger container, or the part applied to every tab (which seems to be the opposite of how things actually are). Does terminology like TabTemplate for the outer, status / tab bar stuff and Viewport (to borrow from @imsnif ) / TabContent / TabBody for the inner part make sense? I'm not sure myself but just wanted to throw out a couple of ideas. I do think that the outer layout does make sense as a "Template" though. I also think this would make more sense to have a template: in the layout.yaml, then a list of content: fields for the inside layouts.
  2. On the topic of the YAML, it could just be my personal aversion to JSON, but things feel like they are getting quite (visually) complex. I couldn't really work out what example/multiple_tabs_layout_htop_command.yaml was doing until I ran it. Something that would be a much bigger change to the way we do layouts (but maybe reduce the nesting a little) would be to have two, totally separate sections of the layout YAML.

As an example, a simplified version of the aforementioned layout:

---
direction: Horizontal
parts:
  - direction: Vertical
    split_size:
      Fixed: 1
    run:
      plugin: tab-bar
  - direction: Vertical
    tabs:
    - direction: Vertical
      parts:
      - direction: Vertical
        split_size:
          Percent: 50
        run:
          command: {cmd: htop}
      - direction: Vertical
        split_size:
          Percent: 50
    - direction: Vertical
    - direction: Vertical
      parts:
      - direction: Vertical
        split_size:
          Percent: 20
        run:
          plugin: strider
      - direction: Horizontal
        split_size:
          Percent: 80
        parts:
          - direction: Vertical
            split_size:
              Percent: 50
          - direction: Vertical
            split_size:
              Percent: 50
  - direction: Vertical
    split_size:
      Fixed: 2
    run:
      plugin: status-bar

Could become something like:

---
direction: Horizontal
tab_template:
  - direction: Vertical
    split_size:
      Fixed: 1
    run:
      plugin: tab-bar
  - direction: Vertical
    body: true
  - direction: Vertical
    split_size:
      Fixed: 2
    run:
      plugin: status-bar
tabs:
  - direction: Vertical
    parts:
    - direction: Vertical
      split_size:
        Percent: 50
      run:
        command: {cmd: htop}
    - direction: Vertical
      split_size:
        Percent: 50
  - direction: Vertical
  - direction: Vertical
    parts:
    - direction: Vertical
      split_size:
        Percent: 20
      run:
        plugin: strider
    - direction: Horizontal
      split_size:
        Percent: 80
      parts:
        - direction: Vertical
          split_size:
            Percent: 50
        - direction: Vertical
          split_size:
            Percent: 50

For me, this makes it visually more obvious that the templated part (appearing on every tab) is a tab bar, body, and status bar, while it's now also clear I'm listing layouts for 3 tabs. It's basically exactly what you are doing already, but it removes the (potentially confusing) nesting / similarity between TabLayouts and generic Layouts.

After my layout changes are merged, this should leave us with the very clean:

---
direction: Horizontal
tab_template:
  - direction: Vertical
    split_size:
      Fixed: 1
    run:
      plugin: tab-bar
  - direction: Vertical
    body: true
  - direction: Vertical
    split_size:
      Fixed: 2
    run:
      plugin: status-bar
tabs:
  - direction: Vertical
    parts:
    - direction: Vertical
      run:
        command: {cmd: htop}
    - direction: Vertical
  - direction: Vertical
  - direction: Vertical
    parts:
    - direction: Vertical
      split_size:
        Percent: 20
      run:
        plugin: strider
    - direction: Horizontal
      parts:
        - direction: Vertical
        - direction: Vertical

As clear as YAML can be, I suppose.

Obviously play around with the naming if you'd like (tab_template could probably just be template), and let me know what you think about that slight shift in how things are written! I find it more obvious, but that could just be me!

* fix(grid): scroll up on empty line

* fix(grid): line wrapping while scrolling

* style(grid): fix names

* docs(changelog): document fix
@a-kenji
Copy link
Contributor Author

a-kenji commented Aug 19, 2021

Thank you for this review @TheLostLambda !
This is very helpful information.
I have to agree with the yaml split, while I think this does not necessarily make it easier in general, I do like the way you can easily add tabs now without having the overhead in thinking about the actual template. So I would like to go that way.

I am not sure about the renaming of the tabs, for now they make sense to me this way.
Maybe have the MainLayout be LayoutTemplate then?

@TheLostLambda
Copy link
Member

Maybe have the MainLayout be LayoutTemplate then?

I think that would be good, I think that Template is a very clear word for this!

I am not sure about the renaming of the tabs, for now they make sense to me this way.

Do you mean the internal structs or the things I wrote in my example as tab_template and tabs? I'm happy to have the internal structs named whatever you find the clearest, though I'll admit I like the tab_template and tabs names – they make it very clear which is the template and tabs is plural (indicating that's the place for listing different tab layouts). I'm happy to compromise on names other than these, though I'd probably like to avoid reusing parts personally.

imsnif and others added 10 commits August 19, 2021 19:02
* fix(performance): undo degredation introduced in 646

* style(fmt): make rustfmt happy

* style(fmt): make clippy happy
* fix(compatibility): properly paste multilines

* test(input): fix bracketed paste assertion
It works as follows:
```
---
template:
  direction: Horizontal
  parts:
    - direction: Vertical
      split_size:
        Fixed: 1
      run:
        plugin: tab-bar
    - direction: Vertical
      body: true
    - direction: Vertical
      split_size:
        Fixed: 2
      run:
        plugin: status-bar
tabs:
  - direction: Vertical
```

The tabs are created in the body section of the template.
…#642)

* Indicate to the user when text is copied to the clipboard

- broadcast CopyToClipboard event to plugins after selection has been
  copied, and InputReceived event after any input has been received.
- add new ClientToServerMsg InputReceived
- subscribe status-bar plugin to new events, modify second line after
  CopyToClipboard, reset it on InputReceived.

* remove unnecessary InputReceived ClientToServerMsg

- use existing Actions instead to know that user input has been received

* make status bar text copied hint bold green

* cleanup

* cleanup

* cleanup
* fix(compatibility): docker-compose progress bar

* docs(changelog): progress bar fix

* style(fmt): make rustfmt happy
…st-tab

feat(tab): add keybind to go to last tab visited
@a-kenji
Copy link
Contributor Author

a-kenji commented Aug 23, 2021

@TheLostLambda
I thought you meant to rename the post_tab and pre_tab, but I think I
understood you wrong. I agree with the other names!

@imsnif
I have been trying to get this ready, but I still have a block in here.
On migrating the tests it seems apparent to me that I didn't change it completely right.
The function LayoutTemplate::split_template() has problems on very deeply nested layouts still.
If you want to check it out, on running the tests there are still 2 layout ones failing.
I will keep on looking into it, but since it blocks work you want to do maybe you see
something here, that Is not currently obvious to me.

I left in the previous working function for a reference still.
It is the Layout::split_main_and_tab_layout()

imsnif and others added 5 commits August 24, 2021 10:58
* adjust example layouts and move them from `./example` to
  `./example/layouts`

* simplify the deserialization of the layout

* layouts are now constructed as follows:

```
---
template:
  direction: Horizontal
  parts:
    - direction: Vertical
      borderless: true
      split_size:
        Fixed: 1
      run:
        plugin: tab-bar
    - direction: Vertical
      body: true # <== The body section specifies the position of the
      # inserted tab
    - direction: Vertical
      borderless: true
      split_size:
        Fixed: 2
      run:
        plugin: status-bar
tabs:
  - direction: Vertical
  - direction: Vertical
```
@a-kenji a-kenji merged commit f802663 into zellij-org:main Aug 26, 2021
@a-kenji a-kenji deleted the tab-layout branch May 10, 2022 06:28
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.

Split layouts into full-layouts and tab-layouts Feature: Open New Tab with A Specific Layout.
6 participants