-
Notifications
You must be signed in to change notification settings - Fork 7
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
make the terminal directive more consistent #34
Conversation
I'm fine with the prompt change. However, I don't understand the problem with the This is definitely not a serious UX issue that warrants breaking the extension. What I would suggest is to make the |
The only problem arises when I'm OK with your suggestion, though, because the confusion mainly arose when seeing these two prompt types used together. In fact, I'd go further and make these two styles mutually exclusive; what do you think? |
I chose the syntax mainly so you wouldn't need to remember two different versions - but also to make sure we don't run into problems where you would need exactly that format in the terminal output. I think
I would document them to be mutually exclusive, but not actually enforce it. Enforcing it would be a breaking change, and I don't see a need for that. |
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.
This is failing now (see the action).
Can you also update the readme?
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.
Looks good, thanks!
Can you increase the version number in setup.cfg
(otherwise I won't be able to publish 😉 )?
done! |
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.
Thanks!
This is a conceptual PR to suggest a breaking change in the behaviour of the
.. terminal
directive. Currently, there are two issues::input:
serves two purposes: it's an option (I suppose this was done to provide a shorthand one-line version of the directive) and a prefix to disambiguate commands from output, which may be confusing for people who are familiar with reSrtucturedText but not this extension.~
), which kinda defies the purpose if the command is supposed to occur elsewhere.The contra part here is obvious: this breaks the directive as it's currently used. Let's discuss.