Autocompleter #362

anselor · 2018-04-18T17:23:39Z

This PR addresses most of the feature described in #349 and adds examples and unit tests exercising the new features.
In a future round of changes, cmd2 will be updated to automatically use AutoCompleter for completion on all argparse-driven commands (unless overridden with a custom completion function)

- Fixed a few bugs in AutoCompleter dealing with nargs='+' or nargs='*' - Adjusted some help output dealing with narg ranges - Fixed spacing problem with printing argument help * examples/tab_autocompletion.py - Removed debug code. - Minor changes.

This is to support installation from package managers on older OSes such as Debian 9.

Added more type hinting to AutoCompleter.

…n 3.4 compatibility)

…forms.

… It works fine when I test by hand.

…completer

…ed function arguments. Added example for fully custom completion functions mixed with argparse/AutoCompleter handling - Also demonstrates the ability to pass in a list, tuple, or dict of parameters to append to the custom completion function. Added new test cases exercising the custom completion function calls. Added AutoCompleter and rl_utils to the coverage report.

… prompt. Re-enabled test cases that were failing due to there not being a terminal during unit tests.

Updated AutoCompleter (#349) to match new directory structure from packaging effort.

…constructor parameters in a more concise/general way. May also resolve the weird Mac issue on Python 3.6

…pletion.

codecov · 2018-04-18T17:29:42Z

Codecov Report

Merging #362 into master will increase coverage by 1.9%.
The diff coverage is 91.47%.

@@            Coverage Diff            @@
##           master     #362     +/-   ##
=========================================
+ Coverage   89.33%   91.24%   +1.9%     
=========================================
  Files           2        3      +1     
  Lines        1876     2273    +397     
=========================================
+ Hits         1676     2074    +398     
+ Misses        200      199      -1

Impacted Files	Coverage Δ
cmd2/rl_utils.py	`100% <100%> (ø)`
cmd2/cmd2.py	`91.26% <100%> (+1.93%)`	⬆️
cmd2/argparse_completer.py	`90.65% <90.65%> (ø)`
cmd2/__init__.py

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7560b0a...df09c85. Read the comment docs.

…e ipy command

tleonhardt

Overall a fabulous PR. I love the new feature.

Address my review comments and I'll take a quick look at it again after you do.

tleonhardt · 2018-04-18T17:33:48Z

main.py

+
+    # Set "use_ipython" to True to include the ipy command if IPython is installed, which supports advanced interactive
+    # debugging of your application via introspection on self.
+    app = Cmd(use_ipython=False)


As long as no unit tests rely on this, then I would recommend setting use_ipython to True because that can aid debugging.

tleonhardt · 2018-04-18T17:34:50Z

tests/test_acargparse.py

+These are primarily tests related to readline completer functions which handle tab-completion of cmd2/cmd commands,
+file system paths, and shell commands.
+
+Copyright 2017 Todd Leonhardt <todd.leonhardt@gmail.com>


You should probably make this Copyright 2018 and yourself ;-)
This comment applies in multiple places.

tleonhardt · 2018-04-18T18:27:07Z

cmd2/AutoCompleter.py

@@ -0,0 +1,796 @@
+# coding=utf-8


In general module file names within Python packages are named using "snake_case" like auto_completer.py

tleonhardt · 2018-04-19T00:49:59Z

.coveragerc

 [run]
 # Source
-source = cmd2.py
+source = cmd2/


Thanks for fixing the code coverage analysis configuration now that cmd2 is a multi-file package

tleonhardt · 2018-04-19T00:51:34Z

cmd2/AutoCompleter.py

+import argparse
+import re as _re
+import sys
+from argparse import OPTIONAL, ZERO_OR_MORE, ONE_OR_MORE, REMAINDER, PARSER, ArgumentError, _


I'm assuming the _ at the end of the line is a mistake and should be deleted?

Nope, they have some sort of function called '_' that they use. I had to import it to override their behavior. Bizarre.

Huh ... Well ain't that special ;-) They win the prize for the single worst function name I have ever seen!!!

Maybe add a comment?

tleonhardt · 2018-04-19T02:31:12Z

cmd2/AutoCompleter.py

+import re as _re
+import sys
+from argparse import OPTIONAL, ZERO_OR_MORE, ONE_OR_MORE, REMAINDER, PARSER, ArgumentError, _
+from .rl_utils import rl_force_redisplay


In general I try to order imports like so:
1st: built-in Python modules
2nd: 3rd-party modules
3rd: Our code
(and each section is alphabetical within that).

So recommend moving import from .rl_utils to end.

tleonhardt · 2018-04-19T02:40:13Z

cmd2/rl_utils.py

+        pass
+
+
+# Check what implementation of readline we are using


Convert comment before class to docstring after

tleonhardt · 2018-04-19T02:41:41Z

docs/requirements.txt

 pyparsing
 pyperclip
 wcwidth
+colorama


Thanks for adding this. We need to keep this up-to-date so that the documentation builds correctly on ReadTheDocs

tleonhardt · 2018-04-19T02:44:27Z

tests/test_autocompletion.py

+    first_match = complete_tester(text, line, begidx, endidx, cmd2_app)
+    assert first_match is not None and \
+           cmd2_app.completion_matches == ['S01E02', 'S01E03', 'S02E01', 'S02E03']
+


Probably don't need quite this many trailing newlines at end of file

tleonhardt · 2018-04-19T02:45:34Z

tox.ini

  wcwidth
 commands =
-  py.test {posargs: -n 2} --cov=cmd2 --cov-report=term-missing --forked
+  py.test {posargs: -n 2} --cov --cov-report=term-missing --forked


Thanks for fixing these

tleonhardt · 2018-04-19T03:22:24Z

examples/tab_autocompletion.py

+    def _query_movie_user_library(self):
+        return TabCompleteExample.USER_MOVIE_LIBRARY
+
+    def _filter_library(self, text, line, begidx, endidx, full, exclude=[]):


WARNING: Mutable default arguments in Python behave like static local variables in C - they are forever persistent and if you change the value within the function, you have permanently altered the default for the next time you call the function.

If this is how you intend this function to work, then please document to make it clear.

If this is NOT how you intend this function to work, then use a non-mutable default argument. If a tuple can work for you then that is fine. If you really need that argument to be a list then you can default it to None and then immediately first thing within the function check if it is None and if so assign an empty list to it.

tleonhardt

Everything looks good to me. Do you know if Kevin plans on reviewing?

tleonhardt · 2018-04-19T16:40:21Z

cmd2/argparse_completer.py

@@ -0,0 +1,821 @@
+# coding=utf-8
+"""
+AutoCompleter interprets the argparse.ArgumentParser internals to automatically


Changes all look good to me.

…ng unit tests due to the lack of a real terminal. Some more comments.

anselor · 2018-04-19T16:54:22Z

No idea if anyone else plans on reviewing this.

tleonhardt · 2018-04-19T16:58:06Z

cmd2/rl_utils.py

+    if not sys.stdout.isatty():
+        return
+
+    if rl_type == RlType.GNU:  # pragma: no cover


This is covered on macOS when the gnureadline module is installed.

The reason I blocked this out is because it can't be covered by the automated tests because there's isn't a terminal.

I guess it depends on where we care about seeing the coverage.

Oh good point. I think it is reasonable to block it out if we know it won't be hit by any of our CI systems.

tleonhardt

Consider removing the "no cover" pragma from the one line which should be covered by our AppVeyor builds.

tleonhardt · 2018-04-19T16:59:00Z

cmd2/rl_utils.py

+        display_fixed = ctypes.c_int.in_dll(readline_lib, "rl_display_fixed")
+        display_fixed.value = 1
+
+    elif rl_type == RlType.PYREADLINE:  # pragma: no cover


This is covered on Windows when the pyreadline module is installed, which is a conditional dependency on Windows so it should be. So this should be covered by our AppVeyor CI builds.

tleonhardt · 2018-04-19T17:56:30Z

@kmvanbrunt I'm good to merge this in whenever. Let me know if you would like to review it or if we should just merge it in.

anselor and others added 28 commits April 12, 2018 13:50

Initial publishing of autocompleter. #349

f29d6af

Merge branch 'python3' into autocompleter

f6b6a3c

Started working on an example for autocompleter usage.

e20f7ec

Merge remote-tracking branch 'origin/master' into autocompleter

874f884

* AutoCompleter

8d4f841

- Fixed a few bugs in AutoCompleter dealing with nargs='+' or nargs='*' - Adjusted some help output dealing with narg ranges - Fixed spacing problem with printing argument help * examples/tab_autocompletion.py - Removed debug code. - Minor changes.

Merge remote-tracking branch 'origin/python3' into autocompleter

bfc190e

Matched changes in the python3 branch.

c99b9a2

Changed setup.py requirement for pyperclip to >= 1.5.27 instead of 1.6.0

21454b2

This is to support installation from package managers on older OSes such as Debian 9.

Added more advanced/complex autocompleter examples.

bb5e358

Added more type hinting to AutoCompleter.

Added unit tests for AutoCompleter.

81adb04

Tweaks for Python 3.4

50c96bc

Try adding typing for Python 3.4

cd60899

More changes for Python 3.4

d5d42fb

Added check for allow_abbrev attribute before accessing it (for Pytho…

07c283e

…n 3.4 compatibility)

Created a common prompt reprint implementation for all supported plat…

dac2680

…forms.

Marked the 2 tests failing the windows unit test as skip for windows.…

dadcd27

… It works fine when I test by hand.

Added common file to provide readline utility functions

9e24c98

Merge branch 'autocompleter' of github.com:python-cmd2/cmd2 into auto…

94da51a

…completer

Bringing back color. Updated dependencies to include colorama.

6585622

Added check for whether the terminal is present before reprinting the…

93cc246

… prompt. Re-enabled test cases that were failing due to there not being a terminal during unit tests.

Merge remote-tracking branch 'origin/master' into autocompleter

a448192

Updated AutoCompleter (#349) to match new directory structure from packaging effort.

Should fix linux import

8a5e2ff

Adding back Pyperclip imports that got mixed up in merge

736cdda

Tweaked AutoCompleter.ACArgumentParser's constructor to pass through …

9452dfa

…constructor parameters in a more concise/general way. May also resolve the weird Mac issue on Python 3.6

Changed coverage to look in cmd2 directory.

2428482

Adjusted some coverage configuration. Found and fixed bug in help com…

94bea93

…pletion.

Adds main.py to run base cmd2 directly for development testing purposes.

154fa93

anselor requested a review from tleonhardt as a code owner April 18, 2018 17:23

tleonhardt requested review from kmvanbrunt and kotfu April 18, 2018 17:31

tleonhardt added enhancement major labels Apr 18, 2018

tleonhardt added this to the 0.9.0 milestone Apr 18, 2018

tleonhardt added 2 commits April 18, 2018 10:46

Updated main.py debug/test app to have execute permissions and includ…

4918300

…e ipy command

Merged ReadTheDocs build fix from master

ff66a95

tleonhardt requested changes Apr 19, 2018

View reviewed changes

tleonhardt reviewed Apr 19, 2018

View reviewed changes

Addresses comments on #362

c218633

tleonhardt previously approved these changes Apr 19, 2018

View reviewed changes

Identified and marked a few blocks of code that can't be reached duri…

df09c85

…ng unit tests due to the lack of a real terminal. Some more comments.

anselor dismissed tleonhardt’s stale review via df09c85 April 19, 2018 16:49

tleonhardt reviewed Apr 19, 2018

View reviewed changes

tleonhardt approved these changes Apr 19, 2018

View reviewed changes

tleonhardt merged commit 49a3290 into master Apr 19, 2018

tleonhardt deleted the autocompleter branch April 19, 2018 21:21

Autocompleter #362

Autocompleter #362

Uh oh!

Conversation

anselor commented Apr 18, 2018

Uh oh!

codecov bot commented Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tleonhardt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tleonhardt Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tleonhardt Apr 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tleonhardt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anselor commented Apr 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tleonhardt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tleonhardt commented Apr 19, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Apr 18, 2018 •

edited

Loading

tleonhardt Apr 18, 2018 •

edited

Loading

tleonhardt Apr 19, 2018 •

edited

Loading