[systemd-devel] Fw: [PATCH 1/2] systemd-helpers
Carlos Morata Castillo
cmc809 at inlumine.ual.es
Sat Jan 10 17:45:30 PST 2015
El Sat, 10 Jan 2015 14:28:26 -0500
Dave Reisner escribió:
>On Sat, Jan 10, 2015 at 01:33:05PM +0100, Carlos Morata Castillo wrote:
>> As stated here, we should use a library for bash autocompletions (maybe even with include guards).
>> Using autotools make the autocompletions to /usr/share/bash-completions/completions (as the exit from pkg-config --variable=completionsdir
>> We ended up having multiple function definitions and even messing around with the global bash function namespace, as the functions are called
>> __get_something being possible to redefine other binary bash function.
>Why the exception for busctl and systemctl?
For two reasons:
1-busctl because is using busctl itself for every completion and being systemd modular maybe the actual host doesn't
have it installed, failed then the autocompletion.
2-systemctl.in. I just left out this file until further review of this approach.
>You aren't actually solving any problems with redefinition in this
>patch. You're actually making the problem worse. Instead of redefining
>*some* common functions on completion load, you're forcing *all* common
>functions in systemd-helpers to be redefined (even if only 1 is used). I
>don't really see this as a problem, though, and you don't explain why
>you seem to think it is.
That's why I mentioned the "include guards" or how you said m4_include would do the job way better.
The purpose is to make a library, i.e:
1-Standarize and don't duplicate code
2-Later on you realize that you need to extend some function, e.g: get_machines to work differently
inside a container or in a host. You just have to change one function not actually each get_machines definition.
Both, main cases for using a library. :)
>How is sourcing this library supposed to work? Each file contains:
> #Common functions
> . ./systemd-helpers
>But that file won't be in $PWD when completions are loaded. Sourcing
>works like any other command -- relative paths will be relative to the
>current process's $PWD, not relative to the disk file calling source.
>Without cd'ing into /usr/share/bash-completions/completions, this
>doesn't work (and please don't cd from completions).
You're totally right.
I fixed it.
echo Doing a;
complete -F _do_a hi
>cp a hi /usr/share/bash-completion/completions/
>hi Doing a
>complete -p hi
complete -F _do_a hi
>I'm not against deduplicating this common code, but you'd be better off
>using m4_include to preprocess the completions at build time.
I agree with you. :)
More information about the systemd-devel