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

Support applying zstyle customizations for perforce_labels and perforce_clients similar to perforce_changes #81

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kting28
Copy link

@kting28 kting28 commented Nov 9, 2021

Hello,
In my use case our perforce server has a lot of clients and labels so calling p4 clients or p4 labels with no arguments is guaranteed to hang the process. The documentation at the top of _perforce mentioned support of zstyle customizations for perforce_changes (e.g. limiting to a particular user). This pull request is just to extend the support to p4 labels and p4 clients so users can limit the range of these instructions through zstyle customizations.

@@ -713,14 +713,15 @@ _perforce_charsets() {

(( $+functions[_perforce_clients] )) ||
_perforce_clients() {
local -a slash cl
local -a slash cl xargs
zstyle -a ":completion:${curcontext}:clients" clients xargs
Copy link
Member

Choose a reason for hiding this comment

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

_perforce_call_p4 foo … will call _call_program foo … which will look up the command style under the foo tag. Thus, instead of

zstyle ':completion:*:p4:*' clients $xargs

one ought to be able to do

zstyle ':completion:*:p4:*:clients' command p4 "${_perforce_global_options[@]}" "${(q)xargs[@]}"

for equivalent functionality. Should we document that rather than add a new (and rather specific) style?

(The extra (q) is because _call_program uses eval-like as opposed to exec-like semantics for its positional arguments.)

Copy link
Member

@danielshahaf danielshahaf Nov 9, 2021

Choose a reason for hiding this comment

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

To be clear, the second code snippet should work without this patch.

EDIT: It won't work at all; see my next comment.

Copy link
Author

@kting28 kting28 Nov 9, 2021

Choose a reason for hiding this comment

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

Got it.
With my patch I'm passing the following zstyle customizations:

zstyle ':completion:*:p4-*:changes' changes -c `p4 -Ztag -F "%clientName%" info`
zstyle ':completion:*:p4-*:clients' clients -u $USER -m 20 
zstyle ':completion:*:p4-*:labels' labels -u $USER -m 20 

I think documenting how to do this without adding new code is ideal. However I'm not clear how to achieve say the -u $USER -m 20 with the snippet you provided. Can you provide an example? Sorry I'm new to this and was just following the _perforce_changes way of handling xargs.

Copy link
Member

@danielshahaf danielshahaf Nov 9, 2021

Choose a reason for hiding this comment

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

However I'm not clear how to achieve say the -u $USER -m 20 with the snippet you provided. Can you provide an example?

Sorry, you're right. It was rather a bad thinko on my part. The global variable won't have been created at the time the zstyle gets set, so a detour through zstyle -e is needed:

zstyle -e ':completion:*:p4-*:clients' command \
        'reply=( p4 "${_perforce_global_options[@]}" -u ${(q)USER} -m 20 )'

The principal change here is adding -e and assigning to $reply; adding the (q) is an independent change.

Note that _perforce_call_p4 clients … is also invoked by _perforce_client_dirs, so the value of the command style under the clients tag would be picked up by that callsite as well. If the two calls of _perforce_call_p4 clients … shouldn't always use the same command as each other, then they shouldn't both be using the same value (in this case, clients) as the "tag" argument of _call_program.

Sorry I'm new to this and was just following the _perforce_changes way of handling xargs.

It's always a good idea to follow prior art elsewhere in the same file. As to _perforce_changes, there are a number of moving parts there: two styles, one of which may be overridden by $NUMERIC under certain conditions. I'm not immediately sure what to suggest there.

@danielshahaf
Copy link
Member

Just noticed that _perforce lists pws as maintainer (line 4). He doesn't seem to have a github account,¹ but we can get in touch with him via -workers@. So, @kting28, if a docs patch is coming up, would it be possible for you to post it to -workers@, so pws sees it? I realize we document that completion patches are welcome on github, but someone who is listed as maintainer deserves a heads up.

¹ On https://github.com/zsh-users/zsh/commits/master his name isn't a hyperlink.

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