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 GoToTabName action to switch tab by name #2120

Merged
merged 9 commits into from
Feb 7, 2023

Conversation

naosense
Copy link
Contributor

@naosense naosense commented Jan 28, 2023

To resolve #2099 , add new action GoToTabName to switch tab by name

@naosense naosense temporarily deployed to cachix January 28, 2023 06:32 — with GitHub Actions Inactive
@naosense naosense temporarily deployed to cachix January 28, 2023 07:53 — with GitHub Actions Inactive
@naosense naosense temporarily deployed to cachix January 28, 2023 09:03 — with GitHub Actions Inactive
@imsnif
Copy link
Member

imsnif commented Jan 30, 2023

Hey @naosense - looks great! I haven't tested this yet (hopefully will get some time later this week) - but for now, could you add a test for this? Maybe something like this? https://github.com/zellij-org/zellij/blob/main/zellij-server/src/unit/screen_tests.rs#L511

@naosense naosense temporarily deployed to cachix January 31, 2023 02:55 — with GitHub Actions Inactive
@naosense
Copy link
Contributor Author

Hey @imsnif , test case added! Please check again.

@naosense naosense temporarily deployed to cachix January 31, 2023 03:19 — with GitHub Actions Inactive
Copy link
Member

@imsnif imsnif left a comment

Choose a reason for hiding this comment

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

The code looks great - I left one minor comment.

Creating a new tab doesn't work for me though... I think this is because we need to also include the new tab layout - as in, we should probably do the whole ScreenInstruction::NewTab thing. I'm not sure this will be trivial here, so if you want to give it a try - that would be great. But I think it's also cool to not implement this flag for now. Your choice.

zellij-server/src/screen.rs Outdated Show resolved Hide resolved
@naosense
Copy link
Contributor Author

naosense commented Feb 6, 2023

@imsnif it's weird, it works for me
image

Can you tell me about your steps, i'll reproduce it and see what's going on

@naosense naosense temporarily deployed to cachix February 6, 2023 02:34 — with GitHub Actions Inactive
@naosense naosense requested a review from imsnif February 6, 2023 02:35
@imsnif
Copy link
Member

imsnif commented Feb 7, 2023

My apologies @naosense - I tested it again and it works. I think I might have forgotten the flag 😅

Copy link
Member

@imsnif imsnif left a comment

Choose a reason for hiding this comment

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

Great work on this!

@imsnif imsnif merged commit 99e8d56 into zellij-org:main Feb 7, 2023
@naosense naosense deleted the switch_tab_by_name branch February 7, 2023 15:43
joshheyse pushed a commit to joshheyse/zellij that referenced this pull request Mar 11, 2023
…#2120)

* Add `GoToTabName` action to switch tab by name

* rm blank file

* add --create option

* format

* add some doc

* add test case

* format

* add test case

* change variable name
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.

Switch to tab by name command
2 participants