dear-imgui.hs icon indicating copy to clipboard operation
dear-imgui.hs copied to clipboard

WIP: Rewrite how arguments are passed

Open ocharles opened this issue 4 years ago • 10 comments

We're not passing all possible arguments to dear-imgui functions. This is my proposal of something lightweight.

ocharles avatar Feb 06 '21 15:02 ocharles

I agree that we need something that emulates the functionality of named parameters, but I find this approach to be rather syntactically heavy.

How are optional fields handled? For instance we have:

DragFloat4(const char* label, float v[4], float v_speed = 1.0f, float v_min = 0.0f, float v_max = 0.0f, const char* format = "%.3f", ImGuiSliderFlags flags = 0);

with this approach we have a record

data DragFloat4 =
  DragFloat4
    { label :: String
    , value :: StateVar (Float, Float, Float, Float)
    , speed, minValue, maxValue :: Float
    , format :: String
    , flags :: ImGuiSliderFlags
    }

now format, speed, minValue, maxValue and flags all have default values, and the others don't.

Generally, I think I would prefer keeping the basic bindings as is (as you argued for in the past), and then provide various convenience wrappers that handle optional arguments and named arguments in extra modules.

sheaf avatar Feb 06 '21 20:02 sheaf

Usually I would supply a default DragFloat4 record and use record updating. The problem is as we use duplicate record fields that won't infer with type annotations :/

ocharles avatar Feb 06 '21 20:02 ocharles

But you'd want a type error if the user doesn't pass in one of the required arguments, no? I don't think that there are sensible default records in general, there are only defaults for a subset of the fields.

sheaf avatar Feb 06 '21 20:02 sheaf

Oh, right, yes. I wouldn't supply defaults for required arguments. This leaves us with:

  • defDragFloat4 :: String -> StateVar ImVec4 -> DragFloat4, so you have to supply the required arguments but the rest are filled in with defaults. Record update is messy here, as you end up with (defDragFloat4 foo bar) { x = y, u = v }, which I find a mess. It also has all the type inference problems due to DuplicateRecordFields
  • All parameters that can have a default can be wrapped in Default a (isomorphic to Maybe a). Then we would have dragFloat4 DragFloat4{ label = "xyz", value = someStateVar, speed = Default, minValue = Default, maxValue = Default, format = Default, flags = Default }. This is quite heavy, though.

It's that, or we're going to have to have two versions of every function with defaults.

Generally, I think I would prefer keeping the basic bindings as is (as you argued for in the past), and then provide various convenience wrappers that handle optional arguments and named arguments in extra modules.

Or maybe you have something else in mind here? Could you sketch out what that would look like for dragFloat4, which seems to be particularly pathological?

ocharles avatar Feb 06 '21 20:02 ocharles

Or maybe you have something else in mind here?

Well my point was more that I can imagine several ways to go about it: using extensible records, using type classes, using overloaded labels, etc. So I thought it would be best not to commit to any one approach, unless we can find a clearly superior solution.

sheaf avatar Feb 06 '21 20:02 sheaf

Ok, but we certainly need to do something. Will have a think. I'm quite reluctant to introduce anything that I'd describe as "novel". How about we have dragFloat4 as is in main and dragFloat4Full :: Drag4Float -> m () which is what's in this PR? dragFloat4 essentially calls dragFloat4Full and fills in the extra fields with defaults. That does seem to give us everything without too much commitment to any particular approach. It's a bit of duplication, but I can live with that.

ocharles avatar Feb 06 '21 21:02 ocharles

I'm trying to whip something up with generic-lens, basically a customised version of the Subtype lens that works with explicitly labelled types instead of record field names.

I've managed to set it up with decent error messages IMO:


data Label ( lbl :: Symbol ) = Label
instance ( lbl' ~ lbl ) => IsLabel lbl ( Label lbl' ) where
  fromLabel = Label
data ( lbl :: Symbol ) := ( a :: Type ) = Label lbl := a

-- | 'project' a smaller type out from a larger one, discarding the rest.
class Project big small where
  project :: big -> small

-- [ Omitted: instances with custom type errors using generic-lens machinery ]

-------------
-- Examples

projOK1 :: ( "b" := Float, "c" := Int, "a" := Double ) -> ( "a" := Double, "b" := Float )
projOK1 = project

projOK2 :: ( "b" := Int, "a" := Float )
projOK2 = project ( #a := 17.7, #b := 9 )

projErr1 :: ( "b" := Float, "a" := Bool, "c" := Double ) -> ( "c" := Double, "b" := Int )
projErr1 = project
-- * 'project': no instance for
--       Project
--         ("b" := Float, "a" := Bool, "c" := Double)
--         ("c" := Double, "b" := Int)
--   The type being projected down is missing the following fields:
--     - #b := Int

projErr2 :: ( "b" := Float, "c" := Int, "a" := Bool ) -> ( "a" := Bool, "b" := Float, "d" := Int )
projErr2 = project
--  * 'project': no instance for
--        Project
--          ("b" := Float, "c" := Int, "a" := Bool)
--          ("a" := Bool, "b" := Float, "d" := Int)
--    The type being projected down is missing the following fields:
--      - #d := Int

projErr3 :: ( "b" := Int, "a" := Int )
projErr3 = project ( #a := 17.7, #b := 9 )
-- * No instance for (Fractional Int) arising from the literal `17.7'

projErr4 :: ( "a" := Bool, "b" := Int, "b" := Int ) -> ( "b" := Int, "b" := Int ) 
projErr4 = project
--  * 'project': no instance for
--        Project
--          ("a" := Bool, "b" := Int, "b" := Int) ("b" := Int, "b" := Int)
--    The following duplicate fields cause a problem:
--      - [ Both] #b := Int

Unfortunately I couldn't directly re-use Subtype as the inference doesn't constrain fields of the same name to have the same type, which would prevent the projOK2 example above from type-checking (because numeric literals are polymorphic).

The above handles projecting down the collection of given arguments to extract the required arguments. To handle optional arguments, there's a similar setup with the following typeclass

-- | 'inject' a smaller type into a large one, overriding the fields
-- in the larger type with those from the smaller type.
class Inject small big where
  inject :: small -> big -> big

With this setup, the type signatures would look like

dragFloat4
  :: forall args m req opt
  .  ( MonadIO m, Project args req, Inject args opt
     , req ~ ( "label" := String, "speed" := Float, "value" := StateVar ( Float, Float, Float, Float ) )
     , opt ~ ( "minValue" := Float, "maxValue" := Float, "format" := String, "flags" := ImGuiSliderFlags )
     )
  => args
  -> m Bool

and the syntax is quite nice:

dragFloat4 ( #label := "hello", #speed := 3.4, #value := valueRef )
dragFloat4 ( #label := "hello", #flags := ImGuiSliderFlags_Logarithmic, #speed := 3.4, #value := valueRef )

Any thoughts? I'll try to polish up the functionality and put it up as a library.

sheaf avatar Feb 07 '21 16:02 sheaf

RecordDotSyntax in GHC 9 enables nicest (and much anticipated) record access syntax. With it, the described approach is possible and very handy. Optional fields are usually handled through Maybe type (which is converted to null).

chekoopa avatar Nov 10 '21 06:11 chekoopa

the described approach is possible and very hand

Do you mean the approach in my PR?

ocharles avatar Nov 10 '21 08:11 ocharles

Do you mean the approach in my PR?

Yes, like that. Though, it suits better for low-level bindings (#61), and concise shortcuts are still in demand.

chekoopa avatar Nov 11 '21 05:11 chekoopa