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 masterstack layout #1556

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

fabian-thomas
Copy link
Contributor

@fabian-thomas fabian-thomas commented Jun 15, 2023

This PR adds the master stack layout as discussed in #1003.

TODOs:

  • add docs
  • check todo in code
  • maybe add options for:
    • position of master node (left, right), currently its always right
    • which node should be focused in the stack when going right from master, currently the bottom node is focused
    • split ratio (currently its 0.6)

@codecov
Copy link

codecov bot commented Jun 18, 2023

Codecov Report

Attention: Patch coverage is 0% with 35 lines in your changes are missing coverage. Please review.

Project coverage is 95.1%. Comparing base (0950774) to head (b4ba291).

❗ Current head b4ba291 differs from pull request most recent head 0bfb8b2. Consider uploading reports for the commit 0bfb8b2 to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #1556     +/-   ##
========================================
- Coverage    95.3%   95.1%   -0.3%     
========================================
  Files         133     133             
  Lines       12054   12089     +35     
========================================
  Hits        11499   11499             
- Misses        555     590     +35     

@t-wissmann
Copy link
Member

I was reluctant to add master stack because a frame in masterstack layout is equivalent to

hc load '(split horizontal:0.6:0 (clients vertical:0) (clients vertical:0 ))' 

(I see that 60% is hardcoded in your PR #1556). I fear that we're duplicating the behaviour of manual tiling because with masterstack implemented, how do you adjust the ratio of 60%?

which node should be focused in the stack when going right from master, currently the bottom node is focused

I found the bottom one surprising when testing your code. Does the middle one make sense?

By the way, the above mentioned manual split simply remembers the node when focusing from left to right.

On the other hand I understand that it's too many keystrokes to split/unsplit manually all the time when switching between one and two clients. Also, I see that many people ask for masterstack so probably I won't object. mergin this pr when it is more mature!

@fabian-thomas
Copy link
Contributor Author

Didn't answer last year for some reason, sorry.

I found the bottom one surprising when testing your code. Does the middle one make sense?

The reason I used the bottom one at that time was that it is the most recent node. Later I switched that to the top node because thats easier for your eyes. Middle would also be a valid choice, but I feel like then it's hard to see which one is the middle one (e.g. when there are 4 nodes in the stack). Best case would probably be remembering the node like it would be for the manual case, but I guess that complicates the code quite a bit.

I have used the current version of the code for the past year, so I would say functionality wise it's solid already. Maybe I can find some time to finish the PR if people are interested in adding the layout upstream. Otherwise I'll just keep using it like it currently is.

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

2 participants