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

A functional version of withShownDefault #16

Closed
pbrisbin opened this issue Nov 1, 2024 · 5 comments
Closed

A functional version of withShownDefault #16

pbrisbin opened this issue Nov 1, 2024 · 5 comments

Comments

@pbrisbin
Copy link
Contributor

pbrisbin commented Nov 1, 2024

I have a custom type for bytes that I use as an option with a better syntax: something like "128m" -> Bytes 128 $ Just M. withShownDefault is perfect for getting default: 128m to appear in the documentation, but its ergonomics were not what I expected. Ultimately, I had to write:

withShownDefault (Bytes 128 $ Just M) (showBytes $ Bytes 128 $ Just M) $ setting
    [ help "Run restylers with --memory"
    , option
    , name "memory"
    , metavar "NUMBER<b|k|m|g>"
    , reader $ eitherReader readBytes
    ]

I have to specify the default value twice, which could lead to bugs. I know I could extract with let or where, of course, but that's not always convenient.

When I first saw withShownDefault I assumed it would work like this:

withShownDefault :: (a -> String) -> a -> Parser a -> Parser a
withShownDefault = undefined

withDefault :: a -> Parser a -> Parser a
withDefault = withShownDefault show

Then I could simply do,

withShownDefault showBytes (Bytes 128 $ Just M) $ setting
    [ help "Run restylers with --memory"
    , option
    , name "memory"
    , metavar "NUMBER<b|k|m|g>"
    , reader $ eitherReader readBytes
    ]

I'm interested in adding a function that works this way, but with the name withShownDefault taken, I'm not sure what to call it.

withShownByDefault :: (a -> String) -> a -> Parser a -> Parser a
withShownByDefault f a = withShownDefault (f a) a

?

@NorfairKing
Copy link
Owner

I think I prefer your version and we should do that, name it withShownDefault and do a major version break.
You can get the old behaviour back with const.
PR welcome.

@pbrisbin
Copy link
Contributor Author

pbrisbin commented Nov 4, 2024

I prefer your version and we should do that

Awesome, should I also change valueWithShown so it's functional in the same way?

@NorfairKing
Copy link
Owner

I prefer your version and we should do that

Awesome, should I also change valueWithShown so it's functional in the same way?

Yes please.
Don't forget a version bump and changelog.
(The date in the changelog is not hugely important.)

@pbrisbin
Copy link
Contributor Author

pbrisbin commented Nov 5, 2024

Opened as #17.

@NorfairKing
Copy link
Owner

Merged manually and released.

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

No branches or pull requests

2 participants