Merge lp:~berdario/quickly/editor-fix into lp:quickly

[Dario Bertini]

if $EDITOR contains a space (e.g. "emacs -nw") calling "quickly edit" would fail

[Didier Roche-Tolomelli (didrocks)]

Hey Dario, thanks for this patch and help to make Quickly better.

You code looks overall good :) However, you can see that we have some automated tests with Quickly, would you mind writing some for this case? (use a dummy EDITOR = cat and cat -option variable)

[lp:~berdario/quickly/editor-fix]

675. By Dario Bertini on 2012-06-29

Added test for "quickly edit"
"env cat" is used as a dummy editor: it has a space to catch the previous bug, and will actually access the files, thus raising an error if quickly will try to open non-existing files

676. By Dario Bertini on 2012-06-29

The test will ignore stderr, so I can't rely on that to make it detect problems
as a workaround, I hardcoded the output of using "ls -1" as the editor, unfortunately it seems it still doesn't work

[Dario Bertini (berdario)]

I spent one hour trying to write a test that will correctly work when "quickly edit" works, and fail when it fails...

I gave up: see the commit message for r676

among the problems I found when writing the test:

-it won't use the local branch's copy of quickly
-it will fail if the script doesn't have a final newline
-sometimes it saves the output in a result.log, sometimes it doesn't
-it will fail if there's a blank line at the end of the script

[Tony Byrne (tony-badwolf)]

Hi Dario
 That was unfortunate. I had a look because writing tests *should* be easy, because it isn't quickly has failed you :(

quickly will use QUICKLY_EDITOR in preference to EDITOR if it is in the environment

so this works in edit.sh for me

QUICKLY_EDITOR="ls -1" quickly edit | grep "__init__"
# ./test_project/__init__.py

piping through grep means I don't have to list every file.

When you call quickly you will always get /usr/bin/quickly unless you change your path

So I use something like this in a makefile when I'm working on a branch
export PATH=path/to/my/quickly/bin:$$PATH

As for the final newline and blank line, it looks to me like a requirement of sed and I think it is just insane. I wrote some patches and tested them with python unittest, the quickly team added python tests capability to the test system to suit me.

[Dario Bertini (berdario)]

it still fails for me... I don't know what is going wrong:

Running test edit.sh...
********************************
--- data/templates/ubuntu-application/test/edit.sh 2012-06-30 12:49:12.060582184 +0200
+++ /home/dario/Projects/quickly/results.log 2012-06-30 12:50:12.432881551 +0200
@@ -14,4 +14,3 @@
 bzr status

 EDITOR="ls -1" quickly edit | grep "__init__"
-# ./test_project/__init__.py
********************************
FAILED

[Dario Bertini (berdario)]

to elaborate on this:
if I run the test with

test/one-test.sh data/templates/ubuntu-application/test/edit.sh

I get the failure, due to the ./test_project/__init__.py line not being printed

if I run it with

bash data/templates/ubuntu-application/test/edit.sh

./test_project/__init__.py is the last line of output as expected

If needed, right now I'm in #quickly on freenode

Unmerged revisions

676. By Dario Bertini on 2012-06-29

The test will ignore stderr, so I can't rely on that to make it detect problems
as a workaround, I hardcoded the output of using "ls -1" as the editor, unfortunately it seems it still doesn't work

675. By Dario Bertini on 2012-06-29

Added test for "quickly edit"
"env cat" is used as a dummy editor: it has a space to catch the previous bug, and will actually access the files, thus raising an error if quickly will try to open non-existing files

674. By Dario Bertini on 2012-06-24

Fixed bug: when $EDITOR contains spaces subprocess fails

=== modified file 'data/templates/ubuntu-application/edit.py'
--- data/templates/ubuntu-application/edit.py	2011-04-27 15:02:41 +0000
+++ data/templates/ubuntu-application/edit.py	2012-06-29 18:23:20 +0000
@@ -65,11 +65,11 @@
 # add helpfile sources
 filelist.extend(glob.glob('help/C/*.page'))
 
-editor = quicklyutils.get_quickly_editors()
+editor = quicklyutils.get_quickly_editors().split()
 if templatetools.in_verbose_mode():
-    instance = subprocess.Popen([editor] + filelist)
+    instance = subprocess.Popen(editor + filelist)
 else:
     nullfile=file("/dev/null")
-    instance = subprocess.Popen([editor] + filelist, stderr=nullfile)
-if editor != 'gedit':
+    instance = subprocess.Popen(editor + filelist, stderr=nullfile)
+if editor[0] != 'gedit':
     instance.wait()
 
=== added file 'data/templates/ubuntu-application/test/edit.sh'
--- data/templates/ubuntu-application/test/edit.sh	1970-01-01 00:00:00 +0000
+++ data/templates/ubuntu-application/test/edit.sh	2012-06-29 18:23:20 +0000
@@ -0,0 +1,25 @@
+#!/bin/sh
+
+cd /tmp
+
+rm -rf test-project*
+
+quickly create ubuntu-application test-project
+# Creating bzr repository and committing
+# Congrats, your new project is setup! cd /tmp/test-project/ to start hacking.
+# Creating project directory test-project
+
+cd test-project
+
+bzr status
+
+EDITOR="ls -1" quickly edit 2>&1
+# help/C/index.page
+# help/C/preferences.page
+# help/C/topic1.page
+# ./test_project/AboutTestProjectDialog.py
+# ./test_project/__init__.py
+# ./test_project/PreferencesTestProjectDialog.py
+# ./test_project/TestProjectWindow.py
+# ./tests/test_example.py
+# ./tests/test_lint.py