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

New modules for creating nested menus #129

Closed
wants to merge 95 commits into from
Closed

New modules for creating nested menus #129

wants to merge 95 commits into from

Conversation

p-implies-q
Copy link

Hey everyone,

I wrote an extension to XMonad that makes it relatively easy to define nested Menus and render them to the screen while moving through them (heavily inspired by Spacemacs and HELM, if anyone is familiar). I thought I'd share the code, since I'm liking this way of interacting with XMonad a lot.

This is my first ever contribution to anything open source, let alone anything Haskell related, so let me know if I dun goofed, or if there are some more steps I should take until this feature could be pulled into XMonad contrib, and I'd be happy to oblige.

sgf-dma and others added 5 commits December 15, 2016 22:13
Extend ManageHook EDSL to work on focused windows and current workspace.
And do not overwrite wm name in `handleFocusQuery`, if user has already set
it.
Move EWMH code from `X.H.Focus` to `X.H.EwmhDesktops`. Thus:

- I'll use `manageHook` for handling activated window.
- By default window activation do nothing (assuming default `ManageHook`).
- I can use `activated` predicate for changing window activation behavior.
- I may use additional combinators from `X.H.Focus` for more complex
  focus/workspace switch strategies.
@pjones
Copy link
Contributor

pjones commented Dec 19, 2016

@p-implies-q Thanks for your PR!

This is definitely an interesting idea. I may be wrong but I believe that XMonad.Prompt can do a lot of what you are doing here. Is there a particular reason you didn't start with that and add the missing features you need?

@p-implies-q
Copy link
Author

I did start with XMonad.Prompt, but as far as I understood (this is the first time that the penny about "What is a Monad and how do I use it" dropped) the way they deal with KeyStrokes is registering them as textual input (with a lot of XMonad.Promt being about how to deal with Tab completion), and once you push enter doing something with the text that you've entered.

Therefore, to use Prompt I would either have had to refactor the whole thing, or live with menus where every option must be followed with an [Enter] keystroke.

I did separate the running of Menus from the rendering of Menus, so I (or someone else) could at some point write some native XMonad code that does the rendering, removing the reliance on Dzen.

@byorgey byorgey changed the title First commit New modules for creating nested menus Dec 21, 2016
strokyl and others added 2 commits December 22, 2016 22:48
When I have multiscreen I think it's usefull to get the next empty workspace
that is not already displayed.
'doing-things-in-the-context-of-the-keyboard', which is defined by withKB and
the various KeyStroke based actions is probably a Monad (since they're supposed
to be 'computational-contexts', if I understand correctly.) I could probably
refactor this into something nicer with that.
Copy link
Member

Choose a reason for hiding this comment

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

If there are certain X actions that should only be run in the context of withKB, then it could make sense to create a monad which is just a newtype wrapper around X:

{-# LANGUAGE GeneralizedNewtypeDeriving #-}

newtype KB a = KB (X a)
  deriving (Functor, Applicative, Monad, blahditty blah blah all the instances that X has)

waitKey :: KB KeyStroke
waitKey = KB $ do  ... etc.

withKB :: KB a -> X (Maybe a)

That way the types make it clear that you have to run these things in the context of a grabbed keyboard (indeed, they make it necessary).

Copy link
Author

Choose a reason for hiding this comment

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

I was trying to implement this today, and all of the stuff you suggested was working out well, until I tried to use cleanMask inside waitKey.


-- | waitKey returns the next KeyStroke that occurs
waitKey :: KB KeyStroke
waitKey = do
  XConf { display=dpy } <- ask
  (sym, e) <- io $
    -- X-stuff to grab the next (Modifiers, KeyPress) from the system
    allocaXEvent $ \e -> do
        maskEvent dpy keyPressMask e
        ev <- getEvent e
        (ks, _) <- lookupString $ asKeyEvent e
        return (fromMaybe xK_VoidSymbol ks, ev)
  -- Remove unused modifiers
  mods <- cleanMask . ev_state $ e
  return (mods, sym)

It now complains that it's getting an X KeyStroke where its expecting a KB KeyStroke. This makes sense to me, but I cannot for the life of me think of a straightforward way around this. This is my first real outing with newtype's, so I was hoping there's a simple solution that I'm simply not aware of. In my understanding, the output of cleanMask lives in the X monad, and since I'm now in the KB monad, I have no way of getting the value out.

Would that mean the type of waitKey has to be KB X KeyStroke ?


This code is taken almost entirely from the key-string parsing capabilities of
EZConfig, written by Brent Yorgey. EZConfig does not export any of these
functions and I did not want to modify EZConfig itself, so I extracted the code,
Copy link
Member

Choose a reason for hiding this comment

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

So I can understand your impulse here, but it's still painful to see so much duplicated code. Let's talk about ideas for reuse. Modifying EZConfig is certainly on the table as long as we don't change its current API. Do you have ideas for how we could unify the two?

Copy link
Author

Choose a reason for hiding this comment

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

Hey Brent,

Thanks for the feedback! This is just a quick message to indicate I've seen and read your comments. I'll ponder things over for a bit and reply in detail in the next few days.

Copy link
Author

Choose a reason for hiding this comment

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

I thought it over for a bit over Christmas, (Merry Christmas! btw :-)).

I looked over all the different modules in xmonad-contrib, and found nearly 20 different one that do something with KeySyms, KeyMasks, etc. Furthermore, there were 3 or 4 slightly different Pretty-Printers for keys, a few 'key-grabbers', and a lot of mappings from keys to usually actions. All of this spread across all the directories of xmonad-contrib.

In my perspective (but feel free to disagree) this seems to indicate that a new module (or set of modules) that unify and ease the use of working with user input through the keyboard wouldn't be uncalled for. Perhaps simply making a new subdirectory "Keys" inside XMonad and populating this with a number of modules, one of which could be the parsing capabilities from "EZConfig.hs", and then simply importing the necessary functions into EZConfig?

I'd be happy to have a go at this refactoring.

@geekosaur
Copy link
Contributor

geekosaur commented Dec 27, 2016 via email

@p-implies-q
Copy link
Author

Thanks for your reply, geekosaur. Your solution didn't work because I don't think X () has a MonadTrans class that is easy to derive. Anyways, my ghc complains about the kindedness not matching when trying to derive. However, I found the solution! I was being an idiot and simply wrapping the entire function in a KB constructor fixed everything...

Will finalize the fixes and push a new version somewhere in the course of today.

@p-implies-q
Copy link
Author

So I refactored both the Menu Modules and EZConfig to move all the Key parsing to XMonad.Keys. Additionally, I made a new KB Monad for actions that must be performed in the context of a grabbed keyboard. There are a lot of other keyboard related things that could be refactored using a centralized Keyboard modules, but that is future-work.

Lemme know what you think.

@p-implies-q
Copy link
Author

Just added imports of Monoid and Applicative to deal with travis failures on old GHC's. (At least, that's why I think it's failing). Why the most recent GHC 8.x is failing, I do not know, but it seems to not be an isolated incident.

@p-implies-q
Copy link
Author

Just waiting for feedback. No hurry, of course. But until I'm informed that steps need to be taken I am letting this lie for a moment. Hope you are all enjoying the downtime.

pjones and others added 3 commits January 4, 2017 14:39
Prompt should have been using getXMonadDir this entire time but since
we now have getXMonadCacheDir use that instead.  This brings
xmonad-contrib inline with the changes in #62.

This also fixes #68
@colonelpanic8
Copy link
Contributor

@p-implies-q any updates

byorgey and others added 12 commits April 19, 2017 12:25
…ng_to_smartpacing

Add ModifySpacing message handling to SmartPacing and SmartSpacingWit…
* Use global state instead of per-layout - so now window is minimized on
  all workspaces (EWMH requires that windows with _NET_WM_STATE_HIDDEN
  set should be minimized on any workspace but previously they were not)
* Use `windows` instead of `modify`. That should fix bugs related to
  actions that should be done by `windows` and not done by
  `modify` (fixes #46)
* Mark module X.H.RestoreMinimized as deprecated
Rewrite minimization-related modules
This is especially useful with the `Tagged' window property.
Add message to "re-partition" a ComboP layout
trivial changes to fix haddock documentation
Before this fix, when using layoutHintsToCenter together with
MultiColumns, in certain situations XMonad would render the border of
the focused window below a border of unfocused windows. This looks odd
and is here fixed by changing MultiColumns to always place the focused
window in front (even though they should not really overlap) and making
LayoutHints preserve the order returned from the underlying layout,
except for the focused window that is placed on top.

This is a good idea since layoutHintsToCenter requires the focused
window to be on top for good rendering, even if that is not really
required when the underlying layout is used on its own. This way
layoutHintsToCenter requires less of the layout that is modified and
MultiColumns is more compatible with future layout modifiers that are
not so considerate.
…derfix

Fix render order of LayoutHints and MultiColumns
@p-implies-q
Copy link
Author

Hey everyone, sorry for the lack of updates. End of PhD stress and lack of discipline. Will be starting this back up.

@pjones
Copy link
Contributor

pjones commented May 26, 2017

Please do not merge master into your feature branch. It makes it nearly impossible to review your changes. Read the contributing guide on how to rebase instead of merging. Thank you.

@slotThe
Copy link
Member

slotThe commented Jan 29, 2021

@p-implies-q are you still interested in pursuing this pr?

The first step would be to rebase on top of master instead of merging master into your branch, as @pjones already said; looking at this right now, I have no idea what your implementation is and what is cruft that's left over from the merge.

@slotThe slotThe removed this from the v0.14 milestone Nov 21, 2021
@slotThe slotThe closed this Mar 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet