Merge pull request #143 from python-cmd2/no_url_load

Refactored to remove a few things that felt out of place

14 files changed, 116 insertions, 127 deletions

diff --git a/.travis.yml b/.travis.yml
index 2d59e8c5..270fe4f7 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -28,25 +28,25 @@ matrix:
 #          python: pypy3
 #          env: TOXENV=pypy3
         # Stock OSX Python
-        - os: osx
-          language: generic
-          env: TOXENV=py27
-        # Latest Python 3.x from Homebrew
-        - os: osx
-          language: generic
-          env:
-            - TOXENV=py36
-            - BREW_INSTALL=python3
+#        - os: osx
+#          language: generic
+#          env: TOXENV=py27
+#        # Latest Python 3.x from Homebrew
+#        - os: osx
+#          language: generic
+#          env:
+#            - TOXENV=py36
+#            - BREW_INSTALL=python3
 install:
-#  - pip install tox
-  - |
-    if [[ $TRAVIS_OS_NAME == 'osx' ]]; then
-      if [[ -n "$BREW_INSTALL" ]]; then
-        brew update
-        brew install "$BREW_INSTALL"
-      fi
-    fi
-    pip install tox
+  - pip install tox
+#  - |
+#    if [[ $TRAVIS_OS_NAME == 'osx' ]]; then
+#      if [[ -n "$BREW_INSTALL" ]]; then
+#        brew update
+#        brew install "$BREW_INSTALL"
+#      fi
+#    fi
+#    pip install tox
 
 script:
   - tox
diff --git a/CHANGES.md b/CHANGES.md
index 213b09d6..b1f34a62 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -8,8 +8,15 @@ News
 
 * Bug fixes
     * Fixed a couple bugs in interacting with pastebuffer/clipboard on macOS and Linux
+    * Fixed a couple bugs in edit and save commands if called when history is empty
 * Enhancements
     * Ensure that path and shell command tab-completion results are alphabetically sorted
+    * Removed feature for load command to load scripts from URLS
+        * It didn't work, there were no unit tests, and it felt out of place
+    * Removed presence of a default file name and default file extension
+        * These also strongly felt out of place
+        * ``load`` and ``_relative_load`` now require a file path
+        * ``edit`` and ``save`` now use a temporary file if a file path isn't provided   
     
 0.7.3
 -----
diff --git a/cmd2.py b/cmd2.py
index 3ae1c599..87140ad4 100755
--- a/cmd2.py
+++ b/cmd2.py
@@ -58,10 +58,6 @@ import six.moves as sm
 # itertools.zip() for Python 2 or zip() for Python 3 - produces an iterator in both cases
 from six.moves import zip
 
-# Python 2 urllib2.urlopen() or Python3  urllib.request.urlopen()
-# noinspection PyUnresolvedReferences
-from six.moves.urllib.request import urlopen
-
 # Python 3 compatibility hack due to no built-in file keyword in Python 3
 # Due to one occurrence of isinstance(<foo>, file) checking to see if something is of file type
 try:
@@ -587,7 +583,6 @@ class Cmd(cmd.Cmd):
     commentInProgress = pyparsing.Literal('/*') + pyparsing.SkipTo(pyparsing.stringEnd ^ '*/')
 
     default_to_shell = False    # Attempt to run unrecognized commands as shell commands
-    defaultExtension = 'txt'    # For ``save``, ``load``, etc.
     excludeFromHistory = '''run r list l history hi ed edit li eof'''.split()
     exclude_from_help = ['do_eof']  # Commands to exclude from the help menu
 
@@ -599,16 +594,14 @@ class Cmd(cmd.Cmd):
     reserved_words = []
     shortcuts = {'?': 'help', '!': 'shell', '@': 'load', '@@': '_relative_load'}
     terminators = [';']
-    urlre = re.compile('(https?://[-\\w./]+)')
 
     # Attributes which ARE dynamically settable at runtime
     abbrev = True  # Abbreviated commands recognized
-    autorun_on_edit = True  # Should files automatically run after editing (doesn't apply to commands)
+    autorun_on_edit = False  # Should files automatically run after editing (doesn't apply to commands)
     case_insensitive = True  # Commands recognized regardless of case
     colors = (platform.system() != 'Windows')
     continuation_prompt = '> '
     debug = False
-    default_file_name = 'command.txt'  # For ``save``, ``load``, etc.
     echo = False
     editor = os.environ.get('EDITOR')
     if not editor:
@@ -628,14 +621,13 @@ class Cmd(cmd.Cmd):
     settable = stubborn_dict('''
         abbrev                Accept abbreviated commands
         autorun_on_edit       Automatically run files after editing
-        case_insensitive      upper- and lower-case both OK
+        case_insensitive      Upper- and lower-case both OK
         colors                Colorized output (*nix only)
         continuation_prompt   On 2nd+ line of input
         debug                 Show full error stack on error
-        default_file_name     for ``save``, ``load``, etc.
         echo                  Echo command issued into output
         editor                Program used by ``edit``
-        feedback_to_output    include nonessentials in `|`, `>` results
+        feedback_to_output    Include nonessentials in `|`, `>` results
         locals_in_py          Allow access to your application in py via self
         prompt                The prompt issued to solicit input
         quiet                 Don't print nonessential feedback
@@ -935,7 +927,8 @@ class Cmd(cmd.Cmd):
             # TODO: Once support for Python 3.x prior to 3.5 is no longer necessary, replace with a real subprocess pipe
 
             # Redirect stdout to a temporary file
-            _, self._temp_filename = tempfile.mkstemp()
+            fd, self._temp_filename = tempfile.mkstemp()
+            os.close(fd)
             self.stdout = open(self._temp_filename, 'w')
         elif statement.parsed.output:
             if (not statement.parsed.outputTo) and (not can_clip):
@@ -1712,7 +1705,7 @@ Edited commands are always run after the editor is closed.
 Edited files are run on close if the ``autorun_on_edit`` settable parameter is True."""
         if not self.editor:
             raise EnvironmentError("Please use 'set editor' to specify your text editing program of choice.")
-        filename = self.default_file_name
+        filename = None
         if arg:
             try:
                 buffer = self._last_matching(int(arg))
@@ -1720,9 +1713,19 @@ Edited files are run on close if the ``autorun_on_edit`` settable parameter is T
                 filename = arg
                 buffer = ''
         else:
-            buffer = self.history[-1]
+            try:
+                buffer = self.history[-1]
+            except IndexError:
+                self.perror('edit must be called with argument if history is empty', traceback_war=False)
+                return
 
+        delete_tempfile = False
         if buffer:
+            if filename is None:
+                fd, filename = tempfile.mkstemp(suffix='.txt', text=True)
+                os.close(fd)
+                delete_tempfile = True
+
             f = open(os.path.expanduser(filename), 'w')
             f.write(buffer or '')
             f.close()
@@ -1732,6 +1735,9 @@ Edited files are run on close if the ``autorun_on_edit`` settable parameter is T
         if self.autorun_on_edit or buffer:
             self.do_load(filename)
 
+        if delete_tempfile:
+            os.remove(filename)
+
     saveparser = (pyparsing.Optional(pyparsing.Word(pyparsing.nums) ^ '*')("idx") +
                   pyparsing.Optional(pyparsing.Word(legalChars + '/\\'))("fname") +
                   pyparsing.stringEnd)
@@ -1742,20 +1748,32 @@ Edited files are run on close if the ``autorun_on_edit`` settable parameter is T
     Usage:  save [N] [file_path]
 
     * N         - Number of command (from history), or `*` for all commands in history (default: last command)
-    * file_path - location to save script of command(s) to (default: value stored in `default_file_name` param)"""
+    * file_path - location to save script of command(s) to (default: value stored in temporary file)"""
         try:
             args = self.saveparser.parseString(arg)
         except pyparsing.ParseException:
             self.perror('Could not understand save target %s' % arg)
             raise SyntaxError(self.do_save.__doc__)
-        fname = args.fname or self.default_file_name
+
+        # If a filename was supplied then use that, otherwise use a temp file
+        if args.fname:
+            fname = args.fname
+        else:
+            fd, fname = tempfile.mkstemp(suffix='.txt', text=True)
+            os.close(fd)
+
         if args.idx == '*':
             saveme = '\n\n'.join(self.history[:])
         elif args.idx:
             saveme = self.history[int(args.idx) - 1]
         else:
-            # Since this save command has already been added to history, need to go one more back for previous
-            saveme = self.history[-2]
+            saveme = ''
+            # Wrap in try to deal with case of empty history
+            try:
+                # Since this save command has already been added to history, need to go one more back for previous
+                saveme = self.history[-2]
+            except IndexError:
+                pass
         try:
             f = open(os.path.expanduser(fname), 'w')
             f.write(saveme)
@@ -1765,43 +1783,13 @@ Edited files are run on close if the ``autorun_on_edit`` settable parameter is T
             self.perror('Error saving {}'.format(fname))
             raise
 
-    def _read_file_or_url(self, fname):
-        """Open a file or URL for reading by the do_load() method.
-
-        This method methodically proceeds in the following path until it succeeds (or fails in the end):
-        1) Try to open the file
-        2) Try to open the URL if it looks like one
-        3) Try to expand the ~ to create an absolute path for the filename
-        4) Try to add the default extension to the expanded path
-        5) Raise an error
-
-        :param fname: str - filename or URL
-        :return: stream  or a file-like object pointing to the file or URL (or raise an exception if it couldn't open)
-        """
-        # TODO: not working on localhost
-        if os.path.isfile(fname):
-            result = open(fname, 'r')
-        else:
-            match = self.urlre.match(fname)
-            if match:
-                result = urlopen(match.group(1))
-            else:
-                fname = os.path.expanduser(fname)
-                try:
-                    result = open(os.path.expanduser(fname), 'r')
-                except IOError:
-                    result = open('%s.%s' % (os.path.expanduser(fname),
-                                             self.defaultExtension), 'r')
-        return result
-
-    def do__relative_load(self, arg=None):
-        """Runs commands in script at file or URL.
+    def do__relative_load(self, file_path):
+        """Runs commands in script file that is encoded as either ASCII or UTF-8 text.
 
-    Usage:  _relative_load [file_path]
+    Usage:  _relative_load <file_path>
 
     optional argument:
-    file_path   a file path or URL pointing to a script
-                default: value stored in `default_file_name` settable param
+    file_path   a file path pointing to a script
 
 Script should contain one command per line, just like command would be typed in console.
 
@@ -1810,38 +1798,43 @@ relative to the already-running script's directory.
 
 NOTE: This command is intended to only be used within text file scripts.
         """
-        if arg:
-            arg = arg.split(None, 1)
-            targetname, args = arg[0], (arg[1:] or [''])[0]
-            targetname = os.path.join(self._current_script_dir or '', targetname)
-            self.do_load('%s %s' % (targetname, args))
+        # If arg is None or arg is an empty string this is an error
+        if not file_path:
+            self.perror('_relative_load command requires a file path:\n', traceback_war=False)
+            return
+
+        file_path = file_path.strip()
+        # NOTE: Relative path is an absolute path, it is just relative to the current script directory
+        relative_path = os.path.join(self._current_script_dir or '', file_path)
+        self.do_load(relative_path)
 
-    def do_load(self, file_path=None):
-        """Runs commands in script at file or URL.
+    def do_load(self, file_path):
+        """Runs commands in script file that is encoded as either ASCII or UTF-8 text.
 
-    Usage:  load [file_path]
+    Usage:  load <file_path>
 
-    * file_path - a file path or URL pointing to a script (default: value stored in `default_file_name` param)
+    * file_path - a file path pointing to a script
 
 Script should contain one command per line, just like command would be typed in console.
         """
-        # If arg is None or arg is an empty string, use the default filename
+        # If arg is None or arg is an empty string this is an error
         if not file_path:
-            targetname = self.default_file_name
-        else:
-            file_path = file_path.split(None, 1)
-            targetname, args = file_path[0], (file_path[1:] or [''])[0].strip()
+            self.perror('load command requires a file path:\n', traceback_war=False)
+            return
+
+        expanded_path = os.path.abspath(os.path.expanduser(file_path.strip()))
         try:
-            target = self._read_file_or_url(targetname)
+            target = open(expanded_path)
         except IOError as e:
-            self.perror('Problem accessing script from %s: \n%s' % (targetname, e))
+            self.perror('Problem accessing script from {}:\n{}'.format(expanded_path, e))
             return
+
         keepstate = Statekeeper(self, ('stdin', 'use_rawinput', 'prompt',
                                        'continuation_prompt', '_current_script_dir'))
         self.stdin = target
         self.use_rawinput = False
         self.prompt = self.continuation_prompt = ''
-        self._current_script_dir = os.path.split(targetname)[0]
+        self._current_script_dir = os.path.dirname(expanded_path)
         stop = self._cmdloop()
         self.stdin.close()
         keepstate.restore()
diff --git a/command.txt b/command.txt
new file mode 100644
index 00000000..e69de29b
--- /dev/null
+++ b/command.txt
diff --git a/docs/settingchanges.rst b/docs/settingchanges.rst
index 94849246..bdb5d186 100644
--- a/docs/settingchanges.rst
+++ b/docs/settingchanges.rst
@@ -104,12 +104,11 @@ with::
 
     (Cmd) set --long
     abbrev: True                   # Accept abbreviated commands
-    autorun_on_edit: True          # Automatically run files after editing
+    autorun_on_edit: False         # Automatically run files after editing
     case_insensitive: True         # upper- and lower-case both OK
     colors: True                   # Colorized output (*nix only)
     continuation_prompt: >         # On 2nd+ line of input
     debug: False                   # Show full error stack on error
-    default_file_name: command.txt # for ``save``, ``load``, etc.
     echo: False                    # Echo command issued into output
     editor: vim                    # Program used by ``edit``
     feedback_to_output: False      # include nonessentials in `|`, `>` results
diff --git a/examples/scripts/nested.txt b/examples/scripts/nested.txt
new file mode 100644
index 00000000..9ec26160
--- /dev/null
+++ b/examples/scripts/nested.txt
@@ -0,0 +1,3 @@
+!echo "Doing a relative load"
+_relative_load script.txt
+ 
diff --git a/examples/transcript_regex.txt b/examples/transcript_regex.txt
index b8e0e654..61bd8838 100644
--- a/examples/transcript_regex.txt
+++ b/examples/transcript_regex.txt
@@ -2,12 +2,11 @@
 # The regex for editor matches any word until first space.  The one for colors is because no color on Windows.
 (Cmd) set
 abbrev: True
-autorun_on_edit: True
+autorun_on_edit: False
 case_insensitive: True
 colors: /(True|False)/
 continuation_prompt: >
 debug: False
-default_file_name: command.txt
 echo: False
 editor: /([^\s]+)/
 feedback_to_output: False
diff --git a/tests/conftest.py b/tests/conftest.py
index 77f525f7..6941d9fc 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -48,12 +48,11 @@ if sys.platform.startswith('win'):
     expect_colors = False
 # Output from the show command with default settings
 SHOW_TXT = """abbrev: True
-autorun_on_edit: True
+autorun_on_edit: False
 case_insensitive: True
 colors: {}
 continuation_prompt: >
 debug: False
-default_file_name: command.txt
 echo: False
 editor: vim
 feedback_to_output: True
@@ -67,20 +66,20 @@ if expect_colors:
     color_str = 'True '
 else:
     color_str = 'False'
-SHOW_LONG = """abbrev: True                   # Accept abbreviated commands
-autorun_on_edit: True          # Automatically run files after editing
-case_insensitive: True         # upper- and lower-case both OK
-colors: {}                  # Colorized output (*nix only)
-continuation_prompt: >         # On 2nd+ line of input
-debug: False                   # Show full error stack on error
-default_file_name: command.txt # for ``save``, ``load``, etc.
-echo: False                    # Echo command issued into output
-editor: vim                    # Program used by ``edit``
-feedback_to_output: True       # include nonessentials in `|`, `>` results
-locals_in_py: True             # Allow access to your application in py via self
-prompt: (Cmd)                  # The prompt issued to solicit input
-quiet: False                   # Don't print nonessential feedback
-timing: False                  # Report execution times
+SHOW_LONG = """
+abbrev: True             # Accept abbreviated commands
+autorun_on_edit: False   # Automatically run files after editing
+case_insensitive: True   # Upper- and lower-case both OK
+colors: {}            # Colorized output (*nix only)
+continuation_prompt: >   # On 2nd+ line of input
+debug: False             # Show full error stack on error
+echo: False              # Echo command issued into output
+editor: vim              # Program used by ``edit``
+feedback_to_output: True # Include nonessentials in `|`, `>` results
+locals_in_py: True       # Allow access to your application in py via self
+prompt: (Cmd)            # The prompt issued to solicit input
+quiet: False             # Don't print nonessential feedback
+timing: False            # Report execution times
 """.format(color_str)
 
 
diff --git a/tests/relative_multiple.txt b/tests/relative_multiple.txt
new file mode 100644
index 00000000..bbd11739
--- /dev/null
+++ b/tests/relative_multiple.txt
@@ -0,0 +1 @@
+_relative_load scripts/one_down.txt 
diff --git a/tests/script.txt b/tests/script.txt
index 1e18262a..4dfe9677 100644
--- a/tests/script.txt
+++ b/tests/script.txt
@@ -1,2 +1 @@
-help
 help history
diff --git a/tests/scripts/one_down.txt b/tests/scripts/one_down.txt
new file mode 100644
index 00000000..b87ff844
--- /dev/null
+++ b/tests/scripts/one_down.txt
@@ -0,0 +1 @@
+_relative_load ../script.txt
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index 4fd9ca1c..5ca95275 100644
--- a/tests/test_cmd2.py
+++ b/tests/test_cmd2.py
@@ -283,18 +283,13 @@ load {}
     assert out == expected
 
 
-def test_base_load_default_file(base_app, capsys):
-    # TODO: Make sure to remove the 'command.txt' file in case it exists
-
+def test_base_load_with_empty_args(base_app, capsys):
     # The way the load command works, we can't directly capture its stdout or stderr
     run_cmd(base_app, 'load')
     out, err = capsys.readouterr()
 
-    # The default file 'command.txt' doesn't exist, so we should get an error message
-    expected = normalize("""ERROR: Problem accessing script from command.txt:
-[Errno 2] No such file or directory: 'command.txt.txt'
-To enable full traceback, run the following command:  'set debug true'
-""")
+    # The load command requires a file path argument, so we should get an error message
+    expected = normalize("""ERROR: load command requires a file path:\n""")
     assert normalize(str(err)) == expected
 
 
@@ -550,10 +545,7 @@ def test_edit_number(base_app):
     run_cmd(base_app, 'edit 1')
 
     # We have an editor, so should expect a system call
-    m.assert_called_once_with('{} {}'.format(base_app.editor, base_app.default_file_name))
-
-    # Editing history item causes a file of default name to get created, remove it so we have a clean slate
-    os.remove(base_app.default_file_name)
+    m.assert_called_once()
 
 
 def test_edit_blank(base_app):
@@ -570,10 +562,7 @@ def test_edit_blank(base_app):
     run_cmd(base_app, 'edit')
 
     # We have an editor, so should expect a system call
-    m.assert_called_once_with('{} {}'.format(base_app.editor, base_app.default_file_name))
-
-    # Editing history item causes a file of default name to get created, remove it so we have a clean slate
-    os.remove(base_app.default_file_name)
+    m.assert_called_once()
 
 
 def test_base_py_interactive(base_app):
diff --git a/tests/test_completion.py b/tests/test_completion.py
index 74cc3d57..a12a4ec2 100644
--- a/tests/test_completion.py
+++ b/tests/test_completion.py
@@ -150,7 +150,7 @@ def test_path_completion_multiple(cmd2_app, request):
     endidx = len(line)
     begidx = endidx - len(text)
 
-    assert cmd2_app.path_complete(text, line, begidx, endidx) == ['script.py', 'script.txt']
+    assert cmd2_app.path_complete(text, line, begidx, endidx) == ['script.py', 'script.txt', 'scripts' + os.path.sep]
 
 def test_path_completion_nomatch(cmd2_app, request):
     test_dir = os.path.dirname(request.module.__file__)
diff --git a/tests/transcript_regex.txt b/tests/transcript_regex.txt
index a44870e9..4cddad2c 100644
--- a/tests/transcript_regex.txt
+++ b/tests/transcript_regex.txt
@@ -2,12 +2,11 @@
 # The regex for editor matches any word until first space.  The one for colors is because no color on Windows.
 (Cmd) set
 abbrev: True
-autorun_on_edit: True
+autorun_on_edit: False
 case_insensitive: True
 colors: /(True|False)/
 continuation_prompt: >
 debug: False
-default_file_name: command.txt
 echo: False
 editor: /([^\s]+)/
 feedback_to_output: True