After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 690515 - tools: bash tab-completion for gst-launch pipelines
tools: bash tab-completion for gst-launch pipelines
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal enhancement
: 1.1.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-12-19 18:17 UTC by David Röthlisberger
Modified: 2013-04-29 19:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tools/gstreamer-completion: Updated to work with the binary registry (2.02 KB, patch)
2012-12-19 18:17 UTC, David Röthlisberger
needs-work Details | Review
tools/gstreamer-completion: Support gst-inspect, and gst-launch element properties (15.10 KB, patch)
2012-12-19 18:18 UTC, David Röthlisberger
none Details | Review
tools/gstreamer-completion: Updated to work with the binary registry (2.08 KB, patch)
2012-12-19 19:48 UTC, David Röthlisberger
none Details | Review
tools/gstreamer-completion: Updated to work with the binary registry (2.12 KB, patch)
2012-12-19 20:12 UTC, David Röthlisberger
committed Details | Review
[PATCH 2/5] tools/gstreamer-completion: Support gst-inspect, and gst-launch element properties (15.00 KB, application/octet-stream)
2012-12-26 13:38 UTC, David Röthlisberger
  Details
[PATCH 2/5] tools/gstreamer-completion: Support gst-inspect, and gst-launch element properties (15.00 KB, patch)
2012-12-26 13:40 UTC, David Röthlisberger
committed Details | Review
[3/5] tools/gstreamer-completion: Bash 3.2 compatibility fixes (9.01 KB, patch)
2012-12-26 13:46 UTC, David Röthlisberger
committed Details | Review
[4/5] tools/gstreamer-completion: Complete option & property values on bash 3.2 (6.40 KB, patch)
2012-12-26 13:50 UTC, David Röthlisberger
committed Details | Review
[5/5] tools/gstreamer-completion: Allow 1.0 and 0.10 scripts installed simultaneously (2.58 KB, patch)
2012-12-26 13:51 UTC, David Röthlisberger
committed Details | Review

Description David Röthlisberger 2012-12-19 18:17:11 UTC
Update the existing completion script to work with the binary registry;
add support for completing gst-launch element properties names and property values.
Comment 1 David Röthlisberger 2012-12-19 18:17:38 UTC
Created attachment 231918 [details] [review]
tools/gstreamer-completion: Updated to work with the binary registry
Comment 2 David Röthlisberger 2012-12-19 18:18:09 UTC
Created attachment 231919 [details] [review]
tools/gstreamer-completion: Support gst-inspect, and gst-launch element properties
Comment 3 David Röthlisberger 2012-12-19 18:19:09 UTC
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 4 Tim-Philipp Müller 2012-12-19 18:35:41 UTC
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?
Comment 5 David Röthlisberger 2012-12-19 19:48:20 UTC
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!
Comment 6 David Röthlisberger 2012-12-19 20:12:19 UTC
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.
Comment 8 David Röthlisberger 2012-12-26 13:38:57 UTC
Created attachment 232238 [details]
[PATCH 2/5] tools/gstreamer-completion: Support gst-inspect, and  gst-launch element properties
Comment 9 David Röthlisberger 2012-12-26 13:40:14 UTC
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.
Comment 10 David Röthlisberger 2012-12-26 13:46:37 UTC
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.
Comment 11 David Röthlisberger 2012-12-26 13:50:44 UTC
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.
Comment 12 David Röthlisberger 2012-12-26 13:51:40 UTC
Created attachment 232242 [details] [review]
[5/5] tools/gstreamer-completion: Allow 1.0 and 0.10 scripts installed simultaneously
Comment 13 Stefan Sauer (gstreamer, gtkdoc dev) 2013-04-22 12:25:25 UTC
David, work great for me. Could you squash the patch into one?
Comment 14 David Röthlisberger 2013-04-22 14:57:32 UTC
> 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. :-)
Comment 15 Dirk Van Haerenborgh 2013-04-23 08:36:29 UTC
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?
Comment 16 Stefan Sauer (gstreamer, gtkdoc dev) 2013-04-29 19:16:23 UTC
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.
Comment 17 Stefan Sauer (gstreamer, gtkdoc dev) 2013-04-29 19:22:28 UTC
Thanks David.