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

Allow customization of help info via introducing semantic Doc #482

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

Conversation

Martinsos
Copy link

@Martinsos Martinsos commented Jun 10, 2023

Should fix #478 once it is done.

@HuwCampbell this is still more of a playground then a solution, and it doesn't even compile completely, but I think it is starting to take some initial shape, so I thought I would rather share while still in this early stage, so you can course-correct me if I am going in a very wrong direction.

I mostly asked questions in the code, in comments, about things that are not clear to me.

The approach is that I introduced a new type of Doc annotation, HelpAnn, which still allows for "old" AnsiStyle annotations (to support future feature of users specifying style via modifiers, and also the current feature of custom AnsiStyle docs they can already provide for some parts of docs like header, footer and similar), but also allows for HelpType annotations, which we use internally to annotate parts.

type HelpDoc = PP.Doc HelpAnn

data HelpAnn = HelpAnnType HelpType | HelpAnnStyle AnsiStyle

data HelpType = CmdName | OptionName | Description | Title | Metavar

The idea is then to later, probably at SimpleDocStream stage (in prettyprinter docs they recommend doing it at this stage), reannotate HelpAnn annotations into AnsiStyle annotations and continue from there same as before.

I haven't yet started at all working on modifiers that would allow users to specify custom style. For now, I just focused on replacing one kind of annotation with another kind of annotation. I mostly got stuck on figuring out where to exactly annotate what with our new semantic annotation (HelpAnn, specifically HelpAnnType), because I am having hard time figuring out which Chunk Doc is presenting what from the code -> is it option name, description, metavar, ... .

Once we have internal semantic annotation working, and a default way defined of converting those annotations (HelpAnn) into ansi annotations (AnsiStyle) (probably we will just do nothing for most of them), we can start looking into allowing users to define their own function that transforms HelpAnn into AnsiStyle as one way of (global) customization, and style modifiers as another way of (local) customization.

Interested to hear what you think, does this direction make sense? Thanks!

@HuwCampbell
Copy link
Collaborator

Hi.

Yes that's pretty much what I was suggesting. Please don't reformat everything in a PR though. It's too hard to tell what the point is.

@Martinsos
Copy link
Author

Hi.

Yes that's pretty much what I was suggesting. Please don't reformat everything in a PR though. It's too hard to tell what the point is.

Ah sorry I didn't realize I was doing reformatting! That was ormolu doing it on its own. I will have to turn it off and then re-do the changes.
In the meantime, do you have any pointers? You said direction seems to be ok, that is great -> what about good places to annotate stuff? I annotated on about two places, but I am not sure about some other places where I should do it. It would help if you could comment on the TODOs that I left in the code.

Comment on lines 112 to 113
[ ( annotateHelp CmdName $ pretty cmdName,
align (annotateHelp Description $ ansiDocToHelpDoc $ extractChunk (infoProgDesc cmdInfo))
Copy link
Author

Choose a reason for hiding this comment

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

I did some annotating here.


with_title :: String -> Chunk HelpDoc -> Chunk HelpDoc
with_title title = fmap (annotateHelp Title . (pretty title .$.))
Copy link
Author

Choose a reason for hiding this comment

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

I did a bit of annotating here.

Comment on lines +68 to +70
annotateHelp Metavar <$> stringChunk (optMetaVar opt)
descs =
map (pretty . showOption) names
map (annotateHelp OptionName . pretty . showOption) names
Copy link
Author

Choose a reason for hiding this comment

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

I did some annotating here.

Comment on lines +114 to +117
[ (annotateHelp CmdName $ pretty cmdName,
align (annotateHelp Description $ ansiDocToHelpDoc $ extractChunk (infoProgDesc cmdInfo))
)
| (cmdName, cmdInfo) <- reverse cmds
Copy link
Author

Choose a reason for hiding this comment

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

Some more annotating here.

Comment on lines +94 to +96
, infoProgDesc :: Chunk AnsiDoc -- ^ brief parser description
, infoHeader :: Chunk AnsiDoc -- ^ header of the full parser description
, infoFooter :: Chunk AnsiDoc -- ^ footer of the full parser description
Copy link
Author

@Martinsos Martinsos Jun 11, 2023

Choose a reason for hiding this comment

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

This is still AnsiDoc, so that existing interface or these definitions doesn't change and so that users can continue using ansi styles while defining these.
Does that make sense?

Comment on lines 154 to 158
, propHelp :: Chunk HelpDoc -- ^ help text for this option
, propMetaVar :: String -- ^ metavariable for this option
, propShowDefault :: Maybe String -- ^ what to show in the help text as the default
, propShowGlobal :: Bool -- ^ whether the option is presented in global options text
, propDescMod :: Maybe ( Doc -> Doc ) -- ^ a function to run over the brief description
, propDescMod :: Maybe ( HelpDoc -> HelpDoc ) -- ^ a function to run over the brief description
Copy link
Author

Choose a reason for hiding this comment

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

I am not quite sure if these should operate on HelpDoc or AnsiDoc?

@Martinsos
Copy link
Author

@HuwCampbell I reverted the formatting changes, so now it should be ready for a closer look!

-- really a "doc chunk"? But isn't Doc already a "chunk of doc"?.

-- TODO: We have two types of functions here: general (Chunk a) operations, and Chunk (Doc a) operations. We should probably split those into separate modules.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's so we can get a different monoid instance and not add spaces or breaks when things are hidden.

You'll find if its removed that internal and hidden options will affect the way the parser is rendered.

@HuwCampbell
Copy link
Collaborator

I spent 10 minutes on the train and got it to this:
image

@HuwCampbell
Copy link
Collaborator

All up your approach it good. Do you mind if I push some commits to your branch?

@Martinsos
Copy link
Author

Martinsos commented Jun 24, 2023

That's looking great! @HuwCampbell sure pls go for it -> I am currently slammed with other work, but might be able to dedicate some more time in a couple of weeks. But if you can make progress / get it done in the meantime, none happier than me!

Btw this is what we currently have in our CLI, and what we would like to also achieve with optparse applicative:

image

So I am really hoping to achieve level of customizability that can bring us close to that.

This is what we achieved with optparse so far:

image

@HuwCampbell
Copy link
Collaborator

Will do. We're still a way to go though.

The issue isn't actually providing colours per se, that's relatively easy. I also want to respect if it's a TTY; environment variables like NOCOLOR; and a --no-color flag – while also giving a good programming API.

I general, if it's not a TTY, one shouldn't print colours; same for the other two ways of suppressing them. This gets a bit tricky for us because at the moment we don't have an environment parser to lean on internally.

@Martinsos
Copy link
Author

Will do. We're still a way to go though.

The issue isn't actually providing colours per se, that's relatively easy. I also want to respect if it's a TTY; environment variables like NOCOLOR; and a --no-color flag – while also giving a good programming API.

I general, if it's not a TTY, one shouldn't print colours; same for the other two ways of suppressing them. This gets a bit tricky for us because at the moment we don't have an environment parser to lean on internally.

Ah interesting, I completely forgot about that. So we are really talking about the support for the ansi escape codes? And you are saying we don't have any logic for checking that at the moment + for propagating it to the right places?

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.

How do I print help info with colored command names?
2 participants