Hi, 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: >> Hi, >> >> As stated here, we should use a library for bash autocompletions (maybe even >> with include guards). >> http://cgit.freedesktop.org/systemd/systemd/commit/shell-completion/bash/localectl?id=a72d698d0d9ff9c158155b44cdc77376df31a317 >> >> Explanation: >> >> Using autotools make the autocompletions to >> /usr/share/bash-completions/completions (as the exit from pkg-config >> --variable=completionsdir >> bash-completion). >> 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. ---------- >cat a #!/bin/bash function a(){ echo Doing a; } >cat hi #!/bin/bash . ${BASH_SOURCE[0]%/*}/a function _do_a(){ a } complete -F _do_a hi >cp a hi /usr/share/bash-completion/completions/ >touch hi >hi Doing a Doing a Doing a 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. > >Cheers, >dR I agree with you. :) Cheers. _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
