GNOME Bugzilla – Bug 690515
tools: bash tab-completion for gst-launch pipelines
Last modified: 2013-04-29 19:34:02 UTC
Update the existing completion script to work with the binary registry; add support for completing gst-launch element properties names and property values.
Created attachment 231918 [details] [review] tools/gstreamer-completion: Updated to work with the binary registry
Created attachment 231919 [details] [review] tools/gstreamer-completion: Support gst-inspect, and gst-launch element properties
The attached patches are against the master branch, and are also available at https://github.com/drothlis/gstreamer/commits/bash-completion-master I also have prepared patches against the 0.10 branch, available at https://github.com/drothlis/gstreamer/commits/bash-completion-0.10
Comment on attachment 231918 [details] [review] tools/gstreamer-completion: Updated to work with the binary registry >The original registry was in xml format (~/.gstreamer-*/registry.xml). >A binary registry format was added in 2007 (commit ebf0c9d3) and made >the default in 2008 (commit 3f39fd7e). It looks like you can still >choose at "configure" time to use the xml registry instead. You can in 0.10, but not in 1.0. >This change to gstreamer-completion should work with either format >because it parses the output of "gst-inspect" instead of reading the >registry file directly. >+_gst_launch() { >+ local cur="${COMP_WORDS[COMP_CWORD]}" >+ COMPREPLY=( $(compgen -W "$(_gst_elements)" -- "$cur") ) >+} && > complete -F _gst_launch -o default gst-launch > >+_gst_elements() { >+ gst-inspect | grep -v 'Total count' | awk -F': +' '{print $2}' This should be gst-inspect-1.0 and gst-launch-1.0 surely?
Created attachment 231925 [details] [review] tools/gstreamer-completion: Updated to work with the binary registry > This should be gst-inspect-1.0 and gst-launch-1.0 surely? Quite right! Sorry, it was an error in the first commit that was fixed in the second commit, which is why I had missed it!
Created attachment 231927 [details] [review] tools/gstreamer-completion: Updated to work with the binary registry Also updated the commit message of the first patch as per Tim's comment #4.
See also http://lists.freedesktop.org/archives/gstreamer-devel/2012-December/038514.html
Created attachment 232238 [details] [PATCH 2/5] tools/gstreamer-completion: Support gst-inspect, and gst-launch element properties
Created attachment 232239 [details] [review] [PATCH 2/5] tools/gstreamer-completion: Support gst-inspect, and gst-launch element properties Since no-one has reviewed this patch yet, I'm replacing it with a patch that has some very small improvements.
Created attachment 232240 [details] [review] [3/5] tools/gstreamer-completion: Bash 3.2 compatibility fixes Bash 3.2 compatibility fixes -- maybe GStreamer 1.0 won't be officially packaged for _Linux_ systems with bash < 4, but OS X does still ship bash 3.2 (and probably always will, because bash 4 is GPL3). Nevertheless it might be good to keep this as a separate commit instead of squashing with the previous patch, so it is easier to revert if we ever want to go back to the simpler, bash4-only implementation.
Created attachment 232241 [details] [review] [4/5] tools/gstreamer-completion: Complete option & property values on bash 3.2 Further bash 3.2 compatibility fixes -- the previous patch works, but doesn't complete option/property _values_ properly due to bash 3 not splitting the command-line-being-completed at "=" characters.
Created attachment 232242 [details] [review] [5/5] tools/gstreamer-completion: Allow 1.0 and 0.10 scripts installed simultaneously
David, work great for me. Could you squash the patch into one?
> Could you squash the patch into one? Any particular reason not to commit it as 5 separate commits? I think that there's a lot of documentation value in the incremental approach taken by the patch series, and in particular in the commit messages; if someone wonders "why is this line like this" they can use `git annotate` and see "oh right, it's for compatibility with bash 3" or whatever. This is particularly important for the `case` statements in `_gst_launch_parse` because they can be rather cryptic. + Patch #1 fixes the previous non-working behaviour, and makes diff #2 clearer. + Patch #2 is the bulk of the implementation. + Patch #3 & #4 are bash 3 compatibility fixes that add a fair amount of complexity. Apart from the above reasons, there is extra value to having them as separate commits, so that we can revert them if we ever choose to drop bash 3 support. If you want atomicity, you could add the 5 commits on a branch and do a git "non-fast-forward" merge (git merge --no-ff) to create a merge commit. Obviously the final decision is with you, the maintainers; but as the author I would prefer that the public commit history reflect my documentation philosophy. Anyway, thanks for looking into this & testing it! I'm excited, my first patches to make it into GStreamer. :-)
Just tested this as well. Works wonderfully! Property completion is a bit slow here, but not so much that it would be annoying. What about installing it to /etc/bash_completion.d/ with make install?
I will push the patches as separate ones. Anyone an opinion about installing it by default. I guess packagers would want to have a configure switch to disable it. We'd need to also check whether bash-completion is installed. I guess we'd look at a patch for doing that, but as this stuff won't change much it is probably not too much of a burden to manually install it for now.
Thanks David.