ceph: bash auto complete for CLI based on mon command descriptions #7693

aclamk · 2016-02-18T09:51:53Z

This is not bind to bash yet. Use as 'ceph --comp osd ls'.
Able to fulfill commands as well as print command line help, when matching is complete.
This is required step before adding hinting of string values, eg. pool names.
Signed-off-by: Adam Kupczyk(a.kupczyk@mirantis.com)

tchaikov · 2016-02-18T10:41:48Z

@aclamk Adam, i assume that this pr supersedes #6264, am i right?

aclamk · 2016-02-18T11:26:12Z

@tchaikov Yes, #6264 is no longer useful.

tchaikov · 2016-02-18T15:39:55Z

src/ceph.in

+        if not matches:
+            continue
+
+        if j < len(sig) and len(args)>0:


@aclamk could you run this script through pep8? i see some of your change does not follow it.

okay. fwiw, you can install it using pip install pep8.

tchaikov · 2016-02-18T16:53:04Z

@aclamk and you might need to revise your commit message with more concise title and a Signed-off-by line, see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#signing-contributions

liewegas · 2016-03-15T13:20:39Z

@aclamk do you mind rebasing this on master to resolve the conflict? thanks!

liewegas · 2016-03-22T14:35:40Z

2016-03-22T07:24:07.627 INFO:teuthology.orchestra.run.smithi024.stderr: File "/bin/ceph", line 523
2016-03-22T07:24:07.627 INFO:teuthology.orchestra.run.smithi024.stderr: print '\n'.join(sorted(set(comps)))
2016-03-22T07:24:07.628 INFO:teuthology.orchestra.run.smithi024.stderr: ^
2016-03-22T07:24:07.628 INFO:teuthology.orchestra.run.smithi024.stderr:SyntaxError: invalid syntax

tchaikov · 2016-03-22T14:38:55Z

src/ceph.in

+    if match_count == 1 and len(comps) == 0:
+        # only one command matched and no hints yet => add help
+        comps = comps + [' ', '#'+match_cmd['help']]
+    print '\n'.join(sorted(set(comps)))


@aclamk might need to fix this.

tchaikov · 2016-03-29T15:08:18Z

src/ceph.in

+        else:
+            # successfully matched all - except last one - arguments
+            if j < len(sig) and len(args) > 0:
+                comps = comps + sig[j].complete(args[-1])


could also put

comps += sig[j].complete(args[-1])

tchaikov · 2016-03-29T15:20:49Z

@aclamk could you please add the bind to bash in "src/bash_completion/ceph", so we can include this feature in jewel?

tchaikov · 2016-03-31T07:38:52Z

src/bash_completion/ceph

 # This is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
 # License version 2.1, as published by the Free Software
-# Foundation.  See file COPYING.
+# Foundation. 


trailing space.

tchaikov · 2016-03-31T07:41:58Z

@aclamk could you

add your "Signed-off-by" to your commit of 4bffbd3, "git commit -s" can do this for you also.
prefix the title of it with "bash_completion: "

?

or you could just squash it into the previous one.

tchaikov · 2016-03-31T07:42:48Z

src/ceph.in

@@ -512,9 +512,9 @@ def complete(sigdict, args, target):
        else:
            # successfully matched all - except last one - arguments
            if j < len(sig) and len(args) > 0:
-                comps = comps + sig[j].complete(args[-1])
+                comps += sig[j].complete(args[-1])


better off moving this change to the previous commit.

tchaikov · 2016-03-31T07:48:30Z

src/bash_completion/ceph

+	#skip this word if it is option
+	if [[ " ${options_noarg} " =~ " ${COMP_WORDS[i]} " ]] ;	then continue; fi
+	#skip this word and next if it is option with arg
+	if [[ " ${options_arg} " =~ " ${COMP_WORDS[i]} " ]] ;   then ((i++)); continue;	fi


could remove the extra spaces? or break the if block into multiple lines.

tchaikov · 2016-03-31T07:49:59Z

tested, works great!

lgtm apart from some formatting nits.

Now logic moved from bash to python. Not bind to bash yet. Use as 'ceph --comp osd ls'. Able to fulfill commands and print command line help. Signed-off-by: Adam Kupczyk <a.kupczyk@mirantis.com>

aclamk · 2016-03-31T08:10:43Z

@tchaikov: added signed-off, reformatted bash code, removed spaces. Please take a look and comment.

tchaikov · 2016-03-31T08:21:40Z

@aclamk could you remove the heading spaces in the commit message of f9591f5

    Signed-off-by: Adam Kupczyk <a.kupczyk@mirantis.com>

and add a prefix of "bash_completion: " to the first line of it. and the commit message is not accurate, imho. we have a bash-completion for ceph CLI, but your change improves it by using the updated "ceph --completion".

Signed-off-by: Adam Kupczyk <a.kupczyk@mirantis.com>

aclamk · 2016-03-31T08:40:07Z

@tchaikov: Its a pleasure to polish out work with you.

tchaikov · 2016-03-31T10:17:42Z

lgtm

tchaikov · 2016-04-01T06:01:19Z

my pleasure also! =)

ErwanAliasr1 · 2016-04-04T14:49:52Z

src/bash_completion/ceph

@@ -1,73 +1,50 @@
 #
 # Ceph - scalable distributed file system
 #
-# Copyright (C) 2011 Wido den Hollander <wido@widodh.nl>


Why do you delete that copyright ?

Since the merge of pr ceph#7693, 'ceph <command>' to get the help is invalid. As a result, 'test/cephtool-test-mon.sh' test was broken This patch simply change the 'ceph <command>' by a 'ceph --help <command>' Since this change the test is passing again. Signed-off-by: Erwan Velu <erwan@redhat.com>

Since the merge of pr ceph#7693, 'ceph command' to get the help is invalid. As a result, 'test/cephtool-test-mon.sh' test was broken This patch simply change the 'ceph command' by a 'ceph --help command' Since this change the test is passing again. Signed-off-by: Erwan Velu <erwan@redhat.com>

aclamk · 2016-04-04T14:58:51Z

@ErwanAliasr1

I deleted copyright intentionally
I was thinking that after change of this magnitude (actually all content new) it would be unfair to leave original copyright
I didnt put my copyright because I felt more along copyleft

ErwanAliasr1 · 2016-04-04T15:14:20Z

@aclamk ok make sense.

Since the merge of pr ceph#7693, 'ceph command' to get the help is invalid. As a result, 'test/cephtool-test-mon.sh' test was broken This patch simply change the 'ceph command' by a 'ceph --help command' Since this change the test is passing again. Signed-off-by: Erwan Velu <erwan@redhat.com>

tchaikov added feature build/ops labels Feb 18, 2016

tchaikov mentioned this pull request Feb 18, 2016

tools: improved bash completion for ceph CLI #6264

Closed

tchaikov reviewed Feb 18, 2016
View reviewed changes

aclamk force-pushed the auto_complete_python branch from 7b7a44e to 89746dc Compare February 19, 2016 10:55

liewegas added this to the jewel milestone Feb 22, 2016

liewegas added wip-sage-testing and removed wip-sage-testing labels Mar 15, 2016

aclamk force-pushed the auto_complete_python branch from 89746dc to 9f7a8a7 Compare March 16, 2016 09:49

liewegas added wip-sage-testing needs-qa labels Mar 21, 2016

liewegas removed the wip-sage-testing label Mar 22, 2016

tchaikov reviewed Mar 22, 2016
View reviewed changes

tchaikov removed the needs-qa label Mar 22, 2016

aclamk force-pushed the auto_complete_python branch from 9f7a8a7 to 6733bde Compare March 24, 2016 08:27

liewegas assigned tchaikov Mar 29, 2016

tchaikov reviewed Mar 29, 2016
View reviewed changes

tchaikov reviewed Mar 31, 2016
View reviewed changes

tools: Auto complete feature for CLI.

a891d94

Now logic moved from bash to python. Not bind to bash yet. Use as 'ceph --comp osd ls'. Able to fulfill commands and print command line help. Signed-off-by: Adam Kupczyk <a.kupczyk@mirantis.com>

aclamk force-pushed the auto_complete_python branch 2 times, most recently from bc8c3bf to f9591f5 Compare March 31, 2016 08:09

bash_completion: now bash completion uses improved "ceph --completion"

81a55d3

Signed-off-by: Adam Kupczyk <a.kupczyk@mirantis.com>

aclamk force-pushed the auto_complete_python branch from f9591f5 to 81a55d3 Compare March 31, 2016 08:39

tchaikov added the needs-qa label Mar 31, 2016

liewegas added the wip-sage-testing label Apr 3, 2016

liewegas changed the title ~~Auto complete feature for CLI. Now logic moved from bash to python.~~ ceph: bash auto complete for CLI based on mon command descriptions Apr 3, 2016

liewegas merged commit dac9ad3 into ceph:master Apr 3, 2016

ErwanAliasr1 reviewed Apr 4, 2016
View reviewed changes

ErwanAliasr1 mentioned this pull request Apr 4, 2016

tests: Fixing broken test/cephtool-test-mon.sh test #8429

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ceph: bash auto complete for CLI based on mon command descriptions #7693

ceph: bash auto complete for CLI based on mon command descriptions #7693

aclamk commented Feb 18, 2016

tchaikov commented Feb 18, 2016

aclamk commented Feb 18, 2016

tchaikov Feb 18, 2016

aclamk Feb 18, 2016

tchaikov Feb 18, 2016

tchaikov commented Feb 18, 2016

liewegas commented Mar 15, 2016

liewegas commented Mar 22, 2016

tchaikov Mar 22, 2016

tchaikov Mar 29, 2016

tchaikov commented Mar 29, 2016

tchaikov Mar 31, 2016

tchaikov commented Mar 31, 2016

tchaikov Mar 31, 2016

tchaikov Mar 31, 2016

tchaikov commented Mar 31, 2016

aclamk commented Mar 31, 2016

tchaikov commented Mar 31, 2016

aclamk commented Mar 31, 2016

tchaikov commented Mar 31, 2016

tchaikov commented Apr 1, 2016

ErwanAliasr1 Apr 4, 2016

aclamk commented Apr 4, 2016

ErwanAliasr1 commented Apr 4, 2016

ceph: bash auto complete for CLI based on mon command descriptions #7693

ceph: bash auto complete for CLI based on mon command descriptions #7693

Conversation

aclamk commented Feb 18, 2016

tchaikov commented Feb 18, 2016

aclamk commented Feb 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tchaikov commented Feb 18, 2016

liewegas commented Mar 15, 2016

liewegas commented Mar 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tchaikov commented Mar 29, 2016

Choose a reason for hiding this comment

tchaikov commented Mar 31, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tchaikov commented Mar 31, 2016

aclamk commented Mar 31, 2016

tchaikov commented Mar 31, 2016

aclamk commented Mar 31, 2016

tchaikov commented Mar 31, 2016

tchaikov commented Apr 1, 2016

Choose a reason for hiding this comment

aclamk commented Apr 4, 2016

ErwanAliasr1 commented Apr 4, 2016