Skip to content

Remove existential types in TUI #1217

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

Open
hasufell opened this issue Jan 18, 2025 · 3 comments
Open

Remove existential types in TUI #1217

hasufell opened this issue Jan 18, 2025 · 3 comments

Comments

@hasufell
Copy link
Member

data FieldInput a b n =
FieldInput
{ inputState :: b -- ^ The state of the input field (what's rendered in the screen)
, inputValidator :: b -> Either ErrorMessage a -- ^ A validator function
, inputHelp :: HelpMessage -- ^ The input helpMessage
, inputRender :: Bool
-> ErrorStatus
-> HelpMessage
-> Label
-> b
-> (Widget n -> Widget n)
-> (Widget n, Maybe (Widget n)) -- ^ How to draw the input and optionally an overlay, with focus a help message and input.
-- A extension function can be applied too
, inputHandler :: BrickEvent n () -> EventM n b () -- ^ The handler
}
makeLensesFor
[ ("inputState", "inputStateL")
, ("inputValidator", "inputValidatorL")
, ("inputName", "inputNameL")
, ("inputHelp", "inputHelpL")
]
''FieldInput
-- | The MenuField is an existential type which stores a Lens' to a part of the Menu state.
-- In also contains a Field input which internal state is hidden
data MenuField s n where
MenuField ::
{ fieldAccesor :: Lens' s a -- ^ A Lens pointing to some part of the state
, fieldInput :: FieldInput a b n -- ^ The input which modifies the state
, fieldLabel :: Label -- ^ The label
, fieldStatus :: ErrorStatus -- ^ Whether the current is valid or not.
, fieldName :: n
} -> MenuField s n
isValidField :: MenuField s n -> Bool
isValidField = (== Valid) . fieldStatus
makeLensesFor
[ ("fieldLabel", "fieldLabelL")
, ("fieldStatus", "fieldStatusL")
]
''MenuField

These types must go. It's impossible to derive or write a lens for fieldInput.

@dfordivam
Copy link
Collaborator

Ideally the need for lenses should go away, along with the existential types.(menuFields :: [MenuField s n] must be removed.)

Other good to have things in the advanced install/compile menus.

  • show user the version which will be chosen by default. The value of the version field should be initialized to this default value
  • a clear button to reset the fields to default values.
  • support for mouse clicks
  • Allow custom design / placement of buttons etc. ( Clear button should be at the bottom, submit at the top.)

@lsmor
Copy link
Collaborator

lsmor commented Mar 31, 2025

TLDR; Removing existentials would require a whole rewrite of the menus, because current implementation models a menu as a Form, i.e. a list of fields. That list must be heterogenous if we want to pair each field in the form with a field in a record type. Other desing options is having two separated structures and match manually input with field in a record. This implies having a bunch of logic for each menu and lack of re-usability

Just a brief understanding of this. That module is a copy/adaptation of Brick's Form widget, but with optics instead of microlenses (Brick uses microlenses). If I remmeber correctly, there was a good reason to do so. I am talking from memory but I thing the argument goes like this

The general argument goes like this

  • we need the user to input many values for some functionalities (like GHC compilation)
  • The most usual framework to do this is via a Form
  • hence, we draw a form in the tui, gather the inputs, validate per-field (and globally too), and translate into some struct

There are two obvious ways of implementing this.

  1. Using Brick's built-in form or something similar
  2. Using a custom form-like logic for each form

I decided to go with the first one because the idea was to have many menus, so we need a way to write those manus easily. The only problem was that Brick uses microlenses, so we needed to add it as a dependency, something undesired back then. Hence, I went for the adaptation of 1. but in reallity we are pretty much doing the same as Brick's Forms.

Let me first summarize how option two would look alike and why it is a bad option IMO, and later explain the existential types

For option two (custom form-like logic) we need to follow the same workflow "take input" -> "validate" -> "build record" so

-- made up types for the sake of example
data GHCCompilationOptions   = {gchversionOption :: ToolVersion,  isolatedpathOption :: Path, ...} -- struct to hold the options
data GHCCompilationInputs  n = {ghcVersionInput :: Edit.Editor T.Text n, isolatedPathIntput :: Edit.Editor T.Text n, ...} -- A copy of the Option Struct but with brick widgets as types, so we can store user input state

-- validators fro each field
validateGHCversion   ::  Edit.Editor T.Text n -> Either ErrorMessage ToolVersion
validateIsolatedPath  ::  Edit.Editor T.Text n -> Either ErrorMessage Path
...

data GHCCompilationMenu n = {
    options :: GHCCompilationOptions
    , brickState :: GHCCompilationInputs
    , ring :: FocusRing n}

-- This is where things get dirty.
handler ::  BrickEvent n () -> EventM n b ()
handler = ...
 -- my guess is that this is way more difficult and error prone with this approach, 
 -- because you have to match every xyzInput with the xyzOption and focus ring
 -- There is no way yo ucan abstrac this away for different ManuOptions-like types,
 -- you have to do for every manu one by one

draw = -- here you face the same problem as with handler

So the option above, implies to define sort of the same logic (handler, drawing, focus ring, etc...) for every menu individually

The option I went for allow to treat every Form as a list of field. So you implement all the logic for a list of fields instead of a record

-- Alternative implementation
data GHCCompilationOptions   = {gchversion :: ToolVersion,  isolatedpath :: Path, ...}
data GHCCompilationInputs  n = {ghcVersionInput :: Edit.Editor T.Text n, isolatedPathIntput :: Edit.Editor T.Text n, ...}

-- Current implementation (sort of)
-- Notice that a and b could be different for each field. a is the type of xyzOption and b is the type of user input
-- Hence, the fields list is herterogenous!!
data Menu s = {state :: s, fields: [ {userInput :: b, handler ::  BrickEvent n () -> EventM n b (), lens :: Lens' s a } ]}

-- example
ghcversionField  =  {
  userInput :: Text -- user inputs text in the tui
  , handler :: BrickEvent n () -> EventM n Text () -- the Tui get events and modify the text
  , lens: Lens' GHCCompilationOption ToolVersion -- this field can watch the tool vesion of the Manu state
}

Does htis makes sense to you? Unfortunatelly, my time is scarce (recently I had a baby) and I can't build a complete example of this. But be aware of the inconvenience triying to remove existential.

@dfordivam
Copy link
Collaborator

@lsmor no worries, I now have a somewhat working prototype to address this, and a few more things regarding the tui widgets implementation.
This new design tries to resolve a few more issues in addition to the [MenuField s n] usage. And this isn't just a rewrite of Menu, but almost the entire tui.

This design does make use of some package, so the existentials are still present.

There is now a proper abstraction to define a widget so that the instances contain both the rendering and event handling logic.

class BaseWidget n a | a -> n where
  draw :: a -> Widget n

  handleEvent
    :: BrickEvent n ()
    -> EventM n a (Maybe HandleEventResult)

  hasOverlay :: a -> Maybe (Some (IsSubWidget n a))
  hasOverlay _ = Nothing

  closeOverlay :: EventM n a ()
  closeOverlay = pure ()

data HandleEventResult = CloseOverlay
  deriving (Eq, Show)

data IsSubWidget n a b where
  IsSubWidget :: (BaseWidget n b) => Lens' a b -> IsSubWidget n a b

The overall structure of the app will be like a tree of widgets where each widget can have an overlay and the nesting can go arbitrarily deep.

The handling of overlay and passing of events around can now be extracted into a separate instance, and removes a decent amount of mess.

instance (BaseWidget n a) => BaseWidget n (BasicOverlay n a) where

data Navigation = Navigation
  { state :: NavigationState
  , overlayOpen :: Maybe (Some (IsSubWidget Common.Name Navigation))
  , tutorial :: BasicOverlay Common.Name Tutorial
  , contextMenu :: BasicOverlay Common.Name ContextMenu
  , ...
  }

The Menu implementation is much now more straightforward with each field as a separate record field. Modifying the state of these will be trivial now.

data GenericMenu n fs r = GenericMenu
  { fields :: fs n
  , focusRing :: F.FocusRing n
  , getOutput :: fs n -> Either T.Text r
  , keyBindings :: Menu.MenuKeyBindings
  ...
  }

data CompileMenuFields n = CompileMenuFields
  { _bootstrapGhc :: EditInput n (Either Version FilePath)
  , _hadrianGhc :: EditInput n (Maybe (Either Version FilePath))
  ...
  } deriving (Generic)

There can be better reuse and composability of smaller widgets. The GenericMenu can be nested to provide features like an edit field inside a select field.

The boilerplate of drawing and event handling of these menu fields will now be handled via instances of Generic.

How does this look @hasufell ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants