Skip to content

New module: X.L.ConditionalLayout #825

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

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

slotThe
Copy link
Member

@slotThe slotThe commented Aug 23, 2023

Supersedes #582

What I haven't done, yet we probably should (not necessarily in this PR):

TODOS:

New module: X.L.ConditionalLayout

Provide conditional variants of 'ModifiedLayout' and 'Choose', so that modifications (specific layouts) are only applied when a particular condition is met.

Co-authored-by: Tomas Janousek [email protected]
Co-authored-by: Ivan Malison [email protected]


Description

Include a description for your changes, including the motivation
behind them.

Checklist

  • I've read CONTRIBUTING.md

  • I've considered how to best test these changes (property, unit,
    manually, ...) and concluded: cursory testing on my computer yielded the desired results, though someone else should look at this.

  • I updated the CHANGES.md file

Provide conditional variants of 'ModifiedLayout' and 'Choose', so that
modifications (specific layouts) are only applied when a particular
condition is met.

Co-authored-by: Tomas Janousek <[email protected]>
Co-authored-by: Ivan Malison <[email protected]>
@slotThe slotThe marked this pull request as draft August 30, 2023 18:46
@slotThe
Copy link
Member Author

slotThe commented Aug 30, 2023

There's a but that I can't figure out right now: Assuming something like

myLayout = onWorkspace "4" (tall ||| Full) $ Full ||| tall ||| Mirror tall
 where
  tall = Tall 1 (3/100) (1/2)

one needs to issue five M-SPC calls to cycle through all layouts, regardless which branch one is on right now (some of those M-SPC invocations will just not do anything—at least not visibly).

Haven't tried debugging this yet, as its getting late, but perhaps someone else immediately knows what's up.

@geekosaur
Copy link
Contributor

Is using Choose internally wise? That's going to add a couple extra mod-space-s and I'm wondering how that interacts with the explicit condition (wouldn't it effectively invert it?).

@geekosaur
Copy link
Contributor

Also it may be better to leave PerScreen alone until we decide what's happening with #813.

conditional :: ModifierCondition c => c -> (l a -> ModifiedLayout m l a) -> (l a -> CondModifiedLayout c m l a)
conditional c ml = CondModifiedLayout True c . ml
conditional :: ModifierCondition c => c -> (l a -> l' a) -> l a -> CondChoose c l' l a
conditional c ml l = CondChoose True c (Choose CL (ml l) l)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we really want this — this gives you two separate instances of l, so whenever the condition flips, you "lose" (as in, no longer observe, rather than losing the actual data) the internal state of l.

If l is something like l1 ||| l2 ||| l3, then the actual Chosen layout might switch every time the condition flips, and also the effect of Shrink/Expand/… only applies to the instance you're looking at.

So I'd say the implementation simplification this provides isn't worth it.

@liskin
Copy link
Member

liskin commented Jun 12, 2025

Also it may be better to leave PerScreen alone until we decide what's happening with #813.

I'm generally in favor of having everything implemented via CondChoose/CondModifiedLayout and making sure these two are perfect (as in, no resource leaks, no stuck decorations, etc.).

(Apologies for disappearing for a couple years. I've been extremely busy, and also extremely burned out. That's not getting better any time soon, but I finally reached a point of needing ConditionalLayout myself, so at least I have some motivation to progress this specific one…)

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.

Display bug with IfMax, PerScreen, PerWorkspace with decorated layouts
3 participants