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

WIP: Support more bind9 utilities #1321

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

WIP: Support more bind9 utilities #1321

wants to merge 7 commits into from

Conversation

pemensik
Copy link

host and nslookup are already supported. But dig is not, similarly with delv and others.

host and nslookup are already supported. But dig is not, similarly with
delv and others.
@akinomyoga akinomyoga requested a review from yedayak January 27, 2025 22:58
@akinomyoga
Copy link
Collaborator

akinomyoga commented Jan 27, 2025

https://gitlab.isc.org/isc-projects/bind9/-/issues/4879

@yedayak seems to have already shown interest in implementing it in the upstream project, and the upstream maintainer seems to be interested in having and maintaining the completions in their repository. If @yedayak has already started working on them, those two versions could be combined. After that, we can submit it to the upstream project.

completions/bind9 Outdated Show resolved Hide resolved
completions/bind9 Outdated Show resolved Hide resolved
completions/bind9 Outdated Show resolved Hide resolved

case $prev in
-c)
_comp_cmd_nslookup__queryclass
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function is defined in another file, which may not be loaded in the session. One needs to "export" the function by renaming _comp_cmd_nslookup__queryclass to _comp_xfunc_nslookup_compgen_queryclass and call it using _comp_compgen -x nslookup queryclass.

This means that we need to modify bash-completion too even if we'd include the completion settings to the upstream.

Copy link
Author

Choose a reason for hiding this comment

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

well, yes. unless we would rely on named-rrchecker providing list of supported types. That does not come with common package containing utilities only.

Copy link
Author

Choose a reason for hiding this comment

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

further more, it might be useful also for ldns drill tool or unbound-host. Even though better would be getting actually supported list from the tools itself, we do not have it always at hand.

But the question is, whether it would not make sense to move nslookup functions into the bind9 as a whole. They are after all bind9 utilities in normal circumstances.

Copy link
Collaborator

@akinomyoga akinomyoga Jan 28, 2025

Choose a reason for hiding this comment

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

But the question is, whether it would not make sense to move nslookup functions into the bind9 as a whole. They are after all bind9 utilities in normal circumstances.

We haven't been officially doing that when there are multiple implementations for a specific command name, but I think it would make sense. We should still keep our current implementation of completions/nslookup at completions/_nslookup for the other implementations of nslookup. We've been regarded our completion/_<cmd> completion settings as "deprecated" completion which should be replaced by the newer implementation of the upstream completion settings, but the present suggestion is to use completion/_<cmd> as the "default" completion settings when a specific completion setting is not provided by the upstream project. Or we might add even another form of the completion files for the "default" completion or another directory for the "default" completion. Thus, your suggestion involves a reconsideration of our framework about the role of our completion/_<cmd> settings or introduction of the mechanism of defining the "default" completion.

completions/bind9 Outdated Show resolved Hide resolved
Partial fixes from review.
completions/bind9 Outdated Show resolved Hide resolved
Adds multiple types specification for dig. It suggests qtypes and query
classes (only uppercase!), known hostnames in addition. Add also quite
similar support for knot's kdig, which is more or less directl dig
alternative.
@yedayak
Copy link
Collaborator

yedayak commented Jan 28, 2025

https://gitlab.isc.org/isc-projects/bind9/-/issues/4879

@yedayak seems to have already shown interest in implementing it in the upstream project, and the upstream maintainer seems to be interested in having and maintaining the completions in their repository. If @yedayak has already started working on them, those two versions could be combined. After that, we can submit it to the upstream project.

I have a start, but it mostly just has sed commands to extract + commands from the help.

Copy link
Collaborator

@yedayak yedayak left a comment

Choose a reason for hiding this comment

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

Currently we always complete the +no* options, which basically duplicates the amount of options offered. What do you think of only completing them once +no is written?

completions/bind9 Outdated Show resolved Hide resolved
completions/bind9 Outdated Show resolved Hide resolved
Except kdig, they have different parameters presentation. Use simple awk
to print parameters offered.
Use more hints from PR, use existing help with specified parameter.
Selected local variables from caller functions are accessible in nested
functions, no need to pass them inside. Pass just remaining parameters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants