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

cli redecorating #76

Merged
merged 14 commits into from
Aug 25, 2023
Merged

cli redecorating #76

merged 14 commits into from
Aug 25, 2023

Conversation

warpfork
Copy link
Collaborator

This set of commits changes the way the helptext is generated for the CLI.

The output is now markdownlike (or, exactly markdown, if called in certain ways), and colorized in the terminal using ANSI escape sequences. Flags galore for this.

The overall text is a bit less compact in most cases, but I think nicely legible.

I've been driving this around on my localhost for a while and grown to like it.

The commit history contains a variety of experiments. Most have been removed again by the end. In general, this was a process of trying something that seemed to have nice bells and whistles, and then finding some glaring issue, and then replacing it with something simpler (admittedly at the cost of taking on more code ourselves). So, some initial attempts to use all-in-one rendering libraries ended up replaced with doing our own goldmark hooks directly.

On the down sides: This heap of code does also raise the complexity of the CLI rigging quite a bit. (This turned out to be a lot more complicated to glue together than I expected at the outset!) Some of that is now fully paid already; some might become ongoing. The most worrying part in terms of maintainability is that slightly different codepaths end up exercised if you use markdown mode and colored markdown-ish mode.

I wrote some test infrastructure to store the markdown help text into the website repo, which seems like it could produce good docs. Having subcommands link to each other when referenced in the docs is still a wishlist item for the future, but should be supportable within the work done here.

@warpfork
Copy link
Collaborator Author

After letting this steep for a while:

  • I don't love this, honestly. The maintainability cost step up is... high.
  • But if we do ditch this... and then try to do any tweaking to the CLI output at all... we'll end up in the same complexity threshhold again rapidly.

The latter is my main argument for merging it. Detangling all this template stuff in the CLI library is unavoidable if we want to tweak it at all. And also the whole markdown parse phase actually frees us from insanity with the golang templating whitespace issues. It's mindboggling that this seems like the "simplest" way forward, but... it kinda does.

Minimal changes vs the upstream so far, other than
cosmetically in the code (I added a helper for heredoc indents),
and a few whitespace tweaks in the output just to prove I can.

This commit just gets it working.  Though it is a bit large,
I think this was about the minimal level of invasion possible.

After this, I think we can start getting more exciting.
Golang's text template system still doesn't make whitespace easy to handle clearly,
but in this case I think legibilty already went up in the templates,
and I got approximately the same result.  (There's a few more breaks than desired
right under the headings; I may or may not chase those.)

I'm moving on to experimenting with attaching markdown processing to the end of this,
which may make a lot of the whitespace conversation change or become moot anyway,
so I won't spend much more time on it here.
This has... mixed success.

I've got three different irons in the fire here:
 1. The plain markdown text coming out (to sanity check my templates)
 2. A library called Glamour used to process it
 3. A brief prototype of going straight to Goldmark and using
  that system's renderers directly on the AST.

I've posted a bunch of notes on this in the dev chat on Matrix already,
as well as some screenshots, so I'll keep this commit note briefer.

In short, controlling the line spacing and indentation is turning out
remarkably hard with option 2.  In fact, I think it's outright
impossible without some invasive changes tantamount to forking.
So that's a bummer.  It got good rapid results and was very exciting!
But "indent the contents under an h4 deeper than under an h2" is just
not a feature I can get there.  And there are a few other weird cliffs
to customizability (links can only be handled one way, except for a
special case around fragment-only links?  tables or other attempts to
pack info densely are limited.  etc) that are unfortunate.

Getting isomorphic html output, if I wanted to include spacing and all,
and have it rendered in html identically to how it would be in plain
ANSI terminal text, is also looking pretty hard with Option 2.
I think we'd end up literally storing the ANSI and all in testmark
codeblocks, and then writing code in the site generator to re-render
that into html.  Possible, and well within the range of things I was
considering doing anyway at the begining of this exploration, but...
also maybe not totally ideal, considering how much control we _could_
have of this, by going with Option 3.

Now, Option 3... well, the big question there is simply "how much work
is that going to be?", and the answer is "I don't know".

But the prototype of using goldmark directly with my own renderer went
off pretty much without a hitch, and it seems like it reaches a very
maximal level of control.  And for reference, the size of goldmark's
own html renderer is about 1000 lines of (very understandable) code.
That's... about a 990 lines more than I'd _like_ to own, but on the
other hand, if it's self contained and doesn't need much tweaking
after the first pass, maybe it's acceptable.

In exchange for that code, we'd get utterly total control of rendering,
so we could fearlessly use all sorts of stuff (say, even codeblocks
within text within headings that describe flags, and get all of it
aligned correctly)... and it would become also totally in reach to
write branches for emitting html color annotations vs ANSI, with full
correctness as the action of both would be driven by the same AST walk.

Jury's still out a bit.  This checkpoint is just that: a checkpoint.
Some of this code will disappear again, for certain.  Which, exactly,
is not yet certain.
... if you have it checked out in the appropriate relative path.
(I'm not yet attempting to introduce any closer coupling of repos.
In the long run, this is probably a great case for submodules again,
or possibly some dogfooding of warpforge's own relationships for
modules in a workspace (if we dare put that level of complex
self-hosting in our own path, which... may not be wise).  But that
won't improve velocity today, so, it's a job for later.)

This uses testmark to maintain the data again.

A few deps are bumped... so that we can use the new testmark suite
system.  That makes it really easy to do a large batch of similar
tests across multiple files.
((Amend: you no longer see that in the go mod diff here because
I've rebased this branch after something else which also forced
that same bump!))

I haven't retrofitted any other existing tests to use the suite system,
but it's almost certainly going to make them simpler and more robust
too.

The CLI help generation is now *all* in markdown, including flags.
I cargo-culted a lot of the stringer there, and added a few very
minor opinionated tweaks of opportunity.  It's probably still all
nonfinal; and in particular I'm still utterly giving up on trying to
get the whitespace I want out of golang templates.

There's a few tweaks to the experimental goldmark code, because I
found... either an interesting edge case, or an outright bug.
Some of our text with parenthesis causes goldmark to produce paragraph
objects that point to themselves as siblings... in a loop.  Oof.
I don't know if that's intended behavior or not, but I doubt it,
because it sure surprised me.  Anyway, for now I work around it.
Our yak-shave depth is already entirely too high right now.

So what _becomes_ of the generated docs hunks?  Well, over in the
site repo, the static site rendering pipeline actually hoists those
hunks back out, and processes them as markdown again.
Isn't that neat?  (There's a "clidocs" tag on the code blocks
which engages that... though you can't see that in this diff; it's
entirely over in the site repo.)

Probably the most "oof" thing in this diff is... I had to disable
the automatic file-level parallelization in the testmark suite system,
because of the dang package scope shenanigans with the CLI lib.
The amount of pain that's producing is already staggeringly high.
... and I think, after playing with it for a good number of hours,
this ain't it either.

lipgloss is coming in at a really weird middle level of abstraction.
The features like margins and wrapping aren't really doing the right
things without an incredible amount of cajoling (and even then).
It seems to have kind of _half_ a component model, and doesn't seem
to compose very gracefully -- it looks like I'll just *keep* passing
width info down on the side, while I use it, which is confusing.
I'm not sure if I'm holding it wrong or if it just doesn't have the
same view as me on what's important.

And it seems to have the helpful opinion that it's gonna emit
CSI reset codes constantly... which absolutely does not compose
with how goldmark AST visitors (or any sane AST visit system) is
going to want to get things done.

Less importantly, but perturbing to me, is that lipgloss is also doing
just a staggering amount of map lookups and memory allocations for
every little thing.

All in all, I think I'm going to have to dive another level deeper,
because this just isn't doing the thing.

(I'm halfway tempted to pull up and go back to using glamour, because
that got an interesting MVP radically faster, but... it just plateaus
so dang hard.  I can't do variable indent; the tables were meh; the
options for doing anything transformative with links was ziltch...
No, I don't think I really wanna rest there either.)

I'm thinking just a few key functions for wrapping text with awareness
of ANSI codes and their (lack of) effect on render size will maybe
be entirely sufficient and do a simpler better job.
A much simpler library (or even just set of short functions; ANSI codes
are distinctly Not That Complicated) for the highlighting will also
totally suffice.  So: that'll be the approach to try up next.
I think we've finally got up to the glamour demo's equivalent,
but also with control over the other things we wanted.

(I'm beginning to regret not simply doing this a LONG time ago, with
drastically fewer indirections, and perhaps not even the markdown bit.
This journey has been FAR longer and sillier than I expected.)

Wordwrap finally works right.  Paragraphs are indented according to
the level of the heading that precedes them.  Finally.

The reflow library works fairly well for the wordwrap.
No complaints other than it's a little complicated in its guts.
And that I'm a bit sad I can't indent in the same pass as wrapping.
But those are small potatoes issues and probably not worth rewriting
that dang thing for, too.

The styling of inline elements within other nodes like paragraphs
is not yet honored; that's still a todo.

I still haven't introduced any links into the markdown, nor
processing for them either here or in the website, but those are
now an obviously reachable distance from this checkpoint.

In the brief custom ANSI code wrangling I banged out, here:
I don't love it, but this honestly seems simpler than not owning it.
I've only bothered to support a few well-known colors in the table
so far, but it's sufficient to proceed with.  Some structs might
be in order for the future if we support rgba stuff; that can come
when it's needed (or, maybe we can look again and have luck finding
a library that does the essentials and not too wildly much more).

I'm dropping glamour and lipgloss from the experients entirely now,
and go.mod shrinks considerably, accordingly.
Suprisingly straightforward.  The goldmark AST API is *really* good.
In the real CLI, it uses the "ansidown" mode.
(I've hardcoded it for now; but surely, it should do some detection
of feature and situation, and be disableable.  Future work.)

In the tests that regenerate the website content: it uses plain
markdown mode.  This gets the content through in a nice form that
let the website pipeline handle redecoration.  And passing through
the whole parser gets us some whitespace regularlization.
Rearranged some linebreaks to work more robustly.
@warpfork
Copy link
Collaborator Author

I'm thinking the generated fixture files should come back to roost in this repo, instead of having the goofy symlink to the hopefully-sibling site repo. We can find another way to get the content in to the site repo. Having the tests in this repo get unreliable if you're working across branches, or doing branched things in the site repo, is rapidly turning out to be bonkers.

…e helptext templates, which was causing panics.
Fixture updates are from blind "-testmark.regen".
@warpfork warpfork merged commit 9d44a58 into master Aug 25, 2023
4 checks passed
@warpfork warpfork deleted the cli-redecorating branch August 25, 2023 16:56
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.

None yet

1 participant