Closed NICHOLAS85 closed 4 years ago
That's an interesting idea!
Possible ways to improve this include:
/bedrock/share/<subsystem>
. For example, brl-fetch
, brl-tutorial
, and pmm
all have drop-in directories there. Instead of having /bedrock/libexec/
be a mix of brl
drop-in and miscellaneous support binaries, we might want to do something like move all /bedrock/libexec/brl-*
entries to /bedrock/share/brl/backends
and make that the back-end. Some parts of the Bedrock code base might have /bedrock/libexec/brl-*
items hard-coded; we'll want to update those accordingly.brl --help
? The easy solution is to just ignore this for non-default subcommands. However, there are ways to make it work if we want to go down that road. For example, brl tutorial --help
auto-detects dropped-in lesson descriptions (although I'd probably use another technique here).brl strat
and stand-alone strat
. We could generalize this and allow drop-in tab completion for drop-in brl
subcommands.If you want to pursue these before the PR is merged, we can work on them. If you think it overkill for your projected use case and would rather get this in with its current scope, I'm fine with that.
- Bedrock already has drop-in support for other subsystems in
/bedrock/share/<subsystem>
. For example,brl-fetch
,brl-tutorial
, andpmm
all have drop-in directories there. Instead of having/bedrock/libexec/
be a mix ofbrl
drop-in and miscellaneous support binaries, we might want to do something like move all/bedrock/libexec/brl-*
entries to/bedrock/share/brl/backends
and make that the back-end. Some parts of the Bedrock code base might have/bedrock/libexec/brl-*
items hard-coded; we'll want to update those accordingly.
Good idea, will begin moving things into /bedrock/share/brl/backends
and fixing hardcoded things accordingly.
- What about
brl --help
? The easy solution is to just ignore this for non-default subcommands. However, there are ways to make it work if we want to go down that road. For example,brl tutorial --help
auto-detects dropped-in lesson descriptions (although I'd probably use another technique here).
I’ll look into how brl tutorial --help
does drop in support. My current idea is to simply list extra available commands at the end(in a nice format of course), however there are ways to support extending --help
with descriptions for extra commands if they provide one. Which solution would you prefer I look into?
- What about tab completion? The easy solution is to just ignore this for non-default subcommands. We could generalize this and allow drop-in tab completion for drop-in
brl
subcommands.
I think I’ve done this before with Zsh completions, I’ll look into it.
If you want to pursue these before the PR is merged, we can work on them. If you think it overkill for your projected use case and would rather get this in with its current scope, I'm fine with that.
I think I’ll work on this and get all your concerns/suggestions worked out. I intended to open this as a draft and have a discussion about this idea but opened as a pull instead. Thanks for the feedback!
Good idea, will begin moving things into
/bedrock/share/brl/backends
and fixing hardcoded things accordingly.
Sounds good
I’ll look into how
brl tutorial --help
does drop in support.
All brl tutorial
backends can be run with a parameter that indicates they should print their corresponding brl tutorial --help
contents. For example, you can run:
$ (. /bedrock/share/common-code && . /bedrock/share/brl-tutorial/lessons/001_basics help)
Bare minimum users are expected to know about Bedrock Linux
to query for the brl tutorial --help
description of the basics lesson. They're also named in the intended brl tutorial --help
list order.
I think that's an adequate system for specifically brl tutorial
where the backends don't have user-facing commands. I'm not sure we want that for brl
, though, as I don't like the idea of intermixing user-facing parameters and internal ones like that.
My current idea is to simply list extra available commands at the end(in a nice format of course), however there are ways to support extending
--help
with descriptions for extra commands if they provide one.
That may certainly be adequate. If you want to go with that, I'm game. However, if we want to get fancy we can probably do better.
Another possibility is for all brl
backends to contain strings (likely in comments) that contain the --help
information. Something like
#!/bedrock/libexec/busybox sh
#
# brl alias
#
# <copyright message>
#
# brl section:5:${color_term}Alias${color_norm} management commands
# brl help:1:${color_norm}Create a ${color_term}stratum${color_norm} ${color_term}alias${color_norm}
where the number indicates brl --help
order information. The order information can be extracted with
order="$(awk -F: '/brl help:/{print $1}')"
and brl
can parse out the actual help content with:
msg="$(awk -F':' '/brl help:/{print $NF}' alias.sh)"
It can then print the message while retaining color information by eval'ing it:
eval "printf \"${msg}\""
It can loop over the sections in the specified order, then for each loop over and print their help output.
That's just off the top of my head; I haven't fully thought this through. There could easily be better ways to do this. For one thing, evals are generally not a good idea, but off the top of my head I don't see a better way to get color information through if we want to retain that. Do feel free to propose completely different alternatives if you see any. I'm also completely game with this being overkill and just appending new items without documentation at the end of brl --help
.
- What about tab completion? The easy solution is to just ignore this for non-default subcommands. We could generalize this and allow drop-in tab completion for drop-in
brl
subcommands.I think I’ve done this before with Zsh completions, I’ll look into it.
Bedrock supports bash
, zsh
, and fish
completion with files you can find in /bedrock/share/{bash,zsh,fish}/completion
. I haven't thought this through at all, but it's probably possible to have their brl
support read its own drop-in subdirectory at something like /bedrock/share/{bash,zsh,fish}/completion/brl.d/<backends>
.
I think I’ll work on this and get all your concerns/suggestions worked out. I intended to open this as a draft and have a discussion about this idea but opened as a pull instead. Thanks for the feedback!
You're welcome, thanks for helping improve Bedrock!
I've begun work on the /bedrock/libexec/brl-*
-> /bedrock/share/brl/backends/brl-*
migration and have some questions.
Would you like to also move brl-fetch
and brl-tutorial
drop-in directories to be under /bedrock/share/brl/
?
For hardcoded brl-*
paths, would you like to keep these hardcoded but pointed at their new locations, or use the /bedrock/bin/brl
frontend? I see you seem to do both in different places.
Since all these backends are going to be in their own dedicated directory do you still want to keep the brl-
prefix?
I had an idea on how to handle help for brl backends. Create a new folder at /bedrock/share/brl/help
. In this folder, a help file for each backend can be created, which contains the help code. The handle_help
function can be slightly rewritten and moved out of each backend file and into the brl
frontend. In the frontend, handle_help
is called and now considers ${2:-}
instead of ${1:-}. If ${2:-}
is -h|--help
print the relevant help message for the backend (at ${1}) from /bedrock/share/brl/help/...
.
I don't see handle_help
used for anything other than brl
stuff so I don't think this would break anything. This detaches the help code from the backend code but means we don't have to do any parsing or mixing of internal and user-facing parameters. brl --help
can then be generated by calls to handle_help and non-interfering internal parameters. What do you think?
That could, of course, be generalized to anywhere you want to use a help message. and instead move help files into /bedrock/share/help
which are organized into their categories from there.
- Would you like to also move
brl-fetch
andbrl-tutorial
drop-in directories to be under/bedrock/share/brl/
?
I don't see why they would be treated any differently than anything else.
- For hardcoded
brl-*
paths, would you like to keep these hardcoded but pointed at their new locations, or use the/bedrock/bin/brl
frontend? I see you seem to do both in different places.
I don't remember my reasoning for any given case off the top of my head. For now, keep the same pattern as currently exists; use the front end when it uses the front end, and use the back end directly when it uses the back ends directly.
- Since all these backends are going to be in their own dedicated directory do you still want to keep the
brl-
prefix?
I have no strong preference here either way.
I had an idea on how to handle help for brl backends. Create a new folder at
/bedrock/share/brl/help
. In this folder, a help file for each backend can be created, which contains the help code. Thehandle_help
function can be slightly rewritten and moved out of each backend file and into thebrl
frontend. In the frontend,handle_help
is called and now considers${2:-}
instead of ${1:-}. If${2:-}
is-h|--help
print the relevant help message for the backend (at ${1}) from/bedrock/share/brl/help/...
.I don't see
handle_help
used for anything other thanbrl
stuff so I don't think this would break anything. This detaches the help code from the backend code but means we don't have to do any parsing or mixing of internal and user-facing parameters.brl --help
can then be generated by calls to handle_help and non-interfering internal parameters. What do you think?
I don't follow how this improves anything. Moving the per-subcommand help code out of the subcommand code just makes it harder to understand the code base.
I think you're confusing brl --help
with brl <subcommand> --help
. brl --help
would need to work differently to handle drop-in subcommands, as it's currently hard coded to the existing set of subcommands. brl <subcommand> --help
is fine as-is and doesn't need any change. Any new dropped in subcommands can handle --help
on their own.
I'll keep stuff as similar as possible regarding the second and third points. I indeed was confusing brl --help
and brl <subcommand> --help
and was somehow making a more complicated solution that wasn't a solution, I'll rethink about that once I get to it.
Pushed my first draft of extending brl --help
. Currently, it parses the information from each backend (in a similar fashion to what paradigm suggested) and sorts their help messages into a section. The sections are predefined at the moment. I have not looked into further sorting after this initial sort yet. It seems to work well, Let me know what you think!
Implemented subsection sorting. The current syntax to add a brl --help
entry is as follows:
# brl help::<section #>::<entry #>::<help message>
Heres brl-alias
as an example:
# brl help::5::1::${color_cmd}alias ${color_norm}Create a ${color_term}stratum${color_norm} ${color_term}alias${color_norm}
The nice thing about the subsection sorting is you can use \<number>\<letter> to squeeze between the current entries if a user wants adds their own backend and help entry.
Implemented drop-in zsh completion. This takes a different approach to how strat
is done. Any user can add a completion for their backend at /bedrock/share/zsh/completion/brl.d/
with the filename formatted as _brl-<backend>
.
In order for your completion to be considered the following must be present as a comment:
# brl comp::<backend>:<short description>
This file is sourced when your backend request completions. Simply follow the format in /bedrock/share/zsh/completion/_brl
to add an argument to be presented. Here's an example completion:
# Located at /bedrock/share/zsh/completion/brl.d/_brl-all
# brl comp::all:Run command in all strata
args+=('(-e --enabled-strata)'{-e,--enabled-strata}'[list enabled strata]')
args+=('(-E --enabled-aliases)'{-E,--enabled-aliases}'[list aliases to enabled strata]')
args+=('(-a --all-strata)'{-a,--all-strata}'[list all strata]')
args+=('(-A --all-aliases)'{-A,--all-aliases}'[list all aliases]')
args+=('(-v --everything)'{-v,--everything}'[equivilent to -Air]')
args+=('(-r --restrict)'{-r,--restrict}'[disable cross-stratum-hooks]')
args+=('(-u --unrestrict)'{-u,--unrestrict}'[do not disable cross-stratum-hooks]')
args+=('(-q --quiet)'{-q,--quiet}'[lesson output verbosity]')
Implemented Bash and Fish drop-in completion support as well as revised Zshs support. Now any file at /bedrock/share/{bash,fish,zsh}/completion/brl.d/
can be used to extend brl
completions. They are sourced, meaning one simply has to follow the same format used in the corresponding main completion file to correctly add a completion.
Zsh drop-in completions no longer require an information string to be considered. However, if a user wants to provide a short description for their backend they can do so by adding the following:
# brl comp::<short description>
Currently figuring out how to automatically populate the initial tab completion with drop-in backends without the need to create a completion file.
I believe this now covers all the points paradigm initially suggested to improve this pr. To summarize:
All brl backends have been moved out of /bedrock/libexec/brl-*
and into /bedrock/share/brl/backends/
and hard-coded paths have been changed accordingly
brl --help
is now extendable with a comment in each backend following this syntax:
# brl help::<section #>::<entry #>::<help message>
The above # brl help::...
tag is used to register a completion entry for each backend
Tab completion can now be extended with entries in /bedrock/share/{bash,fish,zsh}/completion/brl.d/
with a name of _brl-<backend>
. These entries are sourced when needed meaning you should have any internal variables and functions available to use in your completion.
Some small fixes for typos
Alright, I think this is ready to be reviewed again.
For some reason, it won't let me re-request a review from @cptpcrd.
For some reason, it won't let me re-request a review from @cptpcrd.
I'm not a Bedrock developer; I just saw this PR and left a review because I thought the [ -f $backend ]
test needed quotes :-). I'm definitely not qualified to review this PR as it currently stands.
This is still on my radar, but a bit behind some other pressing things scheduled for the 0.7.18 cycle. Once that's out I'll review this and potentially work with you to queue it up for 0.7.19beta1.
Allows any
brl-
prefixed script dropped into/bedrock/libexec/
to be run bybrl
, allowing a user to extendbrl
easily.