-
Notifications
You must be signed in to change notification settings - Fork 1
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
fix: Use show-line-numbers instead of line-numbers #5
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just change the allowed options from line_numbers
to show_line_numbers
.
if thats the case then that means this is an issue with how handling boolean flags should work because then the |
Just fixed the boolean options with e76bf9f, hopefully you can try merge with main and test the suggested changes. |
It works! But now I'm a bit worried about options that are inside tables, since it doesn't seem like they're handled with this change (e.g. |
oh, thats a good catch. |
that should be fixed by 471ab9e now. |
I've also tried rewriting |
Ah! I've forgotten that tables also prepend the current table key to the parameter. I think that yours actually work right now! (Though currently the config and test still refers to |
I believe that it currently fails to call the command since it can parse the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, the latest changes seem to work and pass all the tests, if you can correct the merge confits we should be good to merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the contribution <3
π Description
Fixes an issue where
line_numbers
would throw an error since the parameter is--show-line-numbers
β Checks