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

Explicit category order #97

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

Conversation

cspotcode
Copy link

Implements #96

WIP to gauge if this will be accepted before I add tests.

Copy link
Owner

@arcanis arcanis left a comment

Choose a reason for hiding this comment

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

Sorry I didn't see this sooner - I think it makes sense. There's just the extra complexity in category format caching that I don't entirely understand?

: null;

formattedCategories.set(category, formattedCategory);

Copy link
Owner

Choose a reason for hiding this comment

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

What are those changes for? 🤔

Copy link
Author

Choose a reason for hiding this comment

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

I see what you mean; this cache is only being read in one place, so the computation of formattedCategory should be moved down to line 458 where it is being used.

Copy link
Author

Choose a reason for hiding this comment

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

I somewhat reverted the changes. I'm not entirely sure what formatMarkdownish is doing. Which is better for commandsByCategories keys: the original category, or the markdown-formatted category?

@cspotcode
Copy link
Author

For both this and #99, do you have any guidance on writing tests? I skimmed the tests and I can see the --help tests. However, I expected to see some snapshots of the --help output. Rather, I see the tests often comparing the output of 2x different invocations to verify that they are identical, but not necessarily to verify that the output is formatted in any particular way. I may be misunderstanding the test philosophy, and I'm hoping that you can offer a sentence or two to point me in the right direction.

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.

2 participants