* [PATCH 1/2] Add tests for org-refile-get-targets
@ 2017-05-15 12:54 Sebastian Reuße
2017-05-15 12:54 ` [PATCH 2/2] org-refile: Fix inconsistency when listing refile targets Sebastian Reuße
2017-05-15 16:38 ` [PATCH 1/2] Add tests for org-refile-get-targets Nicolas Goaziou
0 siblings, 2 replies; 15+ messages in thread
From: Sebastian Reuße @ 2017-05-15 12:54 UTC (permalink / raw)
To: emacs-orgmode; +Cc: Sebastian Reuße
* testing/lisp/test-org.el: Add test.
---
testing/examples/refile/a.org | 6 ++++++
testing/examples/refile/b.org | 6 ++++++
testing/lisp/test-org.el | 44 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 56 insertions(+)
create mode 100644 testing/examples/refile/a.org
create mode 100644 testing/examples/refile/b.org
diff --git a/testing/examples/refile/a.org b/testing/examples/refile/a.org
new file mode 100644
index 000000000..7ed235a52
--- /dev/null
+++ b/testing/examples/refile/a.org
@@ -0,0 +1,6 @@
+* a/1/1
+** a/1/2
+*** a/1/3
+* a/2/1
+** a/2/2
+*** a/2/3
diff --git a/testing/examples/refile/b.org b/testing/examples/refile/b.org
new file mode 100644
index 000000000..15359b16e
--- /dev/null
+++ b/testing/examples/refile/b.org
@@ -0,0 +1,6 @@
+* b/1/1
+** b/1/2
+*** b/1/3
+* b/2/1
+** b/2/2
+*** b/2/3
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index b8bd88957..81ec378c2 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -6282,6 +6282,50 @@ (ert-deftest test-org/show-set-visibility ()
(org-show-set-visibility 'minimal)
(org-invisible-p2))))
+\f
+;;; org-refile
+
+(ert-deftest test-org/org-refile-get-targets ()
+ "Test `org-refile-get-targets'."
+ (save-window-excursion
+ (let ((examples-dir (file-truename "../examples/refile/")))
+ (cd examples-dir)
+ (find-file-read-only "a.org")
+ (find-file-read-only "b.org")
+ (rename-buffer "gratuitous-prefix/b.org")
+ (let ((org-refile-targets '((("a.org" "b.org") :level . 2)))
+ (testcases
+ `((nil . ("a/1/2"
+ "a/2/2"
+ "b/1/2"
+ "b/2/2"))
+ (file . ("a.org"
+ "a.org/a\\/1\\/1/a\\/1\\/2"
+ "a.org/a\\/2\\/1/a\\/2\\/2"
+ "b.org"
+ "b.org/b\\/1\\/1/b\\/1\\/2"
+ "b.org/b\\/2\\/1/b\\/2\\/2"))
+ (full-file-path . ,(mapcar (lambda (s) (concat examples-dir s))
+ '("a.org"
+ "a.org/a\\/1\\/1/a\\/1\\/2"
+ "a.org/a\\/2\\/1/a\\/2\\/2"
+ "b.org"
+ "b.org/b\\/1\\/1/b\\/1\\/2"
+ "b.org/b\\/2\\/1/b\\/2\\/2")))
+ (buffer-name . ("a.org"
+ "a.org/a\\/1\\/1/a\\/1\\/2"
+ "a.org/a\\/2\\/1/a\\/2\\/2"
+ "gratuitous-prefix/b.org"
+ "gratuitous-prefix/b.org/b\\/1\\/1/b\\/1\\/2"
+ "gratuitous-prefix/b.org/b\\/2\\/1/b\\/2\\/2")))))
+ (cl-loop for (use-outline-path . expected-targets) in testcases
+ do (let ((org-refile-use-outline-path use-outline-path))
+ (should
+ (equal
+ (mapcar #'car
+ (org-refile-get-targets))
+ expected-targets))))))))
+
(provide 'test-org)
--
2.13.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/2] org-refile: Fix inconsistency when listing refile targets
2017-05-15 12:54 [PATCH 1/2] Add tests for org-refile-get-targets Sebastian Reuße
@ 2017-05-15 12:54 ` Sebastian Reuße
2017-05-17 12:54 ` Nicolas Goaziou
2017-06-21 7:05 ` Allen Li
2017-05-15 16:38 ` [PATCH 1/2] Add tests for org-refile-get-targets Nicolas Goaziou
1 sibling, 2 replies; 15+ messages in thread
From: Sebastian Reuße @ 2017-05-15 12:54 UTC (permalink / raw)
To: emacs-orgmode; +Cc: Sebastian Reuße
* org.el (org-refile-get-targets): Setting org-refile-use-outline-path
to `file' or `buffer-name' causes an additional target for the file’s
root node to be inserted. This functionality was absent when using
`full-file-path'. We now add this since it is convenient and makes the
behavior more consistent.
---
lisp/org.el | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lisp/org.el b/lisp/org.el
index 97713c523..28277e352 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11563,6 +11563,8 @@ (defun org-refile-get-targets (&optional default-buffer)
(push (list (file-name-nondirectory f) f nil nil) tgs))
(when (eq org-refile-use-outline-path 'buffer-name)
(push (list (buffer-name (buffer-base-buffer)) f nil nil) tgs))
+ (when (eq org-refile-use-outline-path 'full-file-path)
+ (push (list (file-truename (buffer-file-name (buffer-base-buffer))) f nil nil) tgs))
(org-with-wide-buffer
(goto-char (point-min))
(setq org-outline-path-cache nil)
--
2.13.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] org-refile: Fix inconsistency when listing refile targets
2017-05-15 12:54 ` [PATCH 2/2] org-refile: Fix inconsistency when listing refile targets Sebastian Reuße
@ 2017-05-17 12:54 ` Nicolas Goaziou
2017-06-21 7:05 ` Allen Li
1 sibling, 0 replies; 15+ messages in thread
From: Nicolas Goaziou @ 2017-05-17 12:54 UTC (permalink / raw)
To: Sebastian Reuße; +Cc: emacs-orgmode
Hello,
Sebastian Reuße <seb@wirrsal.net> writes:
> * org.el (org-refile-get-targets): Setting org-refile-use-outline-path
> to `file' or `buffer-name' causes an additional target for the file’s
> root node to be inserted. This functionality was absent when using
> `full-file-path'. We now add this since it is convenient and makes the
> behavior more consistent.
Applied. Thank you.
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] org-refile: Fix inconsistency when listing refile targets
2017-05-15 12:54 ` [PATCH 2/2] org-refile: Fix inconsistency when listing refile targets Sebastian Reuße
2017-05-17 12:54 ` Nicolas Goaziou
@ 2017-06-21 7:05 ` Allen Li
2017-06-21 7:55 ` Sebastian Reuße
1 sibling, 1 reply; 15+ messages in thread
From: Allen Li @ 2017-06-21 7:05 UTC (permalink / raw)
To: Sebastian Reuße; +Cc: emacs-orgmode
On Mon, May 15, 2017 at 5:54 AM, Sebastian Reuße <seb@wirrsal.net> wrote:
> * org.el (org-refile-get-targets): Setting org-refile-use-outline-path
> to `file' or `buffer-name' causes an additional target for the file’s
> root node to be inserted. This functionality was absent when using
> `full-file-path'. We now add this since it is convenient and makes the
> behavior more consistent.
> ---
> lisp/org.el | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/lisp/org.el b/lisp/org.el
> index 97713c523..28277e352 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -11563,6 +11563,8 @@ (defun org-refile-get-targets (&optional default-buffer)
> (push (list (file-name-nondirectory f) f nil nil) tgs))
> (when (eq org-refile-use-outline-path 'buffer-name)
> (push (list (buffer-name (buffer-base-buffer)) f nil nil) tgs))
> + (when (eq org-refile-use-outline-path 'full-file-path)
> + (push (list (file-truename (buffer-file-name (buffer-base-buffer))) f nil nil) tgs))
> (org-with-wide-buffer
> (goto-char (point-min))
> (setq org-outline-path-cache nil)
> --
> 2.13.0
>
>
If I'm not mistaken, the full file path needs to be wrapped in
org-protect-slash?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] org-refile: Fix inconsistency when listing refile targets
2017-06-21 7:05 ` Allen Li
@ 2017-06-21 7:55 ` Sebastian Reuße
2017-06-21 8:01 ` Allen Li
0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Reuße @ 2017-06-21 7:55 UTC (permalink / raw)
To: Allen Li; +Cc: emacs-orgmode
Hello Allen,
Allen Li <vianchielfaura@gmail.com> writes:
> On Mon, May 15, 2017 at 5:54 AM, Sebastian Reuße <seb@wirrsal.net> wrote:
>> * org.el (org-refile-get-targets): Setting org-refile-use-outline-path
>> to `file' or `buffer-name' causes an additional target for the file’s
>> root node to be inserted. This functionality was absent when using
>> `full-file-path'. We now add this since it is convenient and makes the
>> behavior more consistent.
>> ---
>> lisp/org.el | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/lisp/org.el b/lisp/org.el
>> index 97713c523..28277e352 100644
>> --- a/lisp/org.el
>> +++ b/lisp/org.el
>> @@ -11563,6 +11563,8 @@ (defun org-refile-get-targets (&optional default-buffer)
>> (push (list (file-name-nondirectory f) f nil nil) tgs))
>> (when (eq org-refile-use-outline-path 'buffer-name)
>> (push (list (buffer-name (buffer-base-buffer)) f nil nil) tgs))
>> + (when (eq org-refile-use-outline-path 'full-file-path)
>> + (push (list (file-truename (buffer-file-name (buffer-base-buffer))) f nil nil) tgs))
>> (org-with-wide-buffer
>> (goto-char (point-min))
>> (setq org-outline-path-cache nil)
>> --
>> 2.13.0
>>
>>
>
> If I'm not mistaken, the full file path needs to be wrapped in
> org-protect-slash?
Slashes that are part of file paths aren’t escaped anymore, as of
53bcf91a9. The only reason to escape slashes is to indicate they are not
part of the outline or file path, but part of the headline instead.
In fact it looks like Nicolas removed ‘org-protect-slash’ in 2b44a1a74
and inlined it at its only use site.
Kind regards,
Sebastian
--
Insane cobra split the wood
Trader of the lowland breed
Call a jittney, drive away
In the slipstream we will stay
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] org-refile: Fix inconsistency when listing refile targets
2017-06-21 7:55 ` Sebastian Reuße
@ 2017-06-21 8:01 ` Allen Li
2017-06-28 22:03 ` Allen Li
0 siblings, 1 reply; 15+ messages in thread
From: Allen Li @ 2017-06-21 8:01 UTC (permalink / raw)
To: Sebastian Reuße; +Cc: emacs-orgmode
Thanks for the info. I was chasing down this bug in org-20170606,
where it still required `org-protect-slash`, so I was anxious about
its absence in your patch.
On Wed, Jun 21, 2017 at 12:55 AM, Sebastian Reuße <seb@wirrsal.net> wrote:
> Hello Allen,
>
> Allen Li <vianchielfaura@gmail.com> writes:
>
>> On Mon, May 15, 2017 at 5:54 AM, Sebastian Reuße <seb@wirrsal.net> wrote:
>>> * org.el (org-refile-get-targets): Setting org-refile-use-outline-path
>>> to `file' or `buffer-name' causes an additional target for the file’s
>>> root node to be inserted. This functionality was absent when using
>>> `full-file-path'. We now add this since it is convenient and makes the
>>> behavior more consistent.
>>> ---
>>> lisp/org.el | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/lisp/org.el b/lisp/org.el
>>> index 97713c523..28277e352 100644
>>> --- a/lisp/org.el
>>> +++ b/lisp/org.el
>>> @@ -11563,6 +11563,8 @@ (defun org-refile-get-targets (&optional default-buffer)
>>> (push (list (file-name-nondirectory f) f nil nil) tgs))
>>> (when (eq org-refile-use-outline-path 'buffer-name)
>>> (push (list (buffer-name (buffer-base-buffer)) f nil nil) tgs))
>>> + (when (eq org-refile-use-outline-path 'full-file-path)
>>> + (push (list (file-truename (buffer-file-name (buffer-base-buffer))) f nil nil) tgs))
>>> (org-with-wide-buffer
>>> (goto-char (point-min))
>>> (setq org-outline-path-cache nil)
>>> --
>>> 2.13.0
>>>
>>>
>>
>> If I'm not mistaken, the full file path needs to be wrapped in
>> org-protect-slash?
>
> Slashes that are part of file paths aren’t escaped anymore, as of
> 53bcf91a9. The only reason to escape slashes is to indicate they are not
> part of the outline or file path, but part of the headline instead.
>
> In fact it looks like Nicolas removed ‘org-protect-slash’ in 2b44a1a74
> and inlined it at its only use site.
>
> Kind regards,
>
> Sebastian
>
> --
> Insane cobra split the wood
> Trader of the lowland breed
> Call a jittney, drive away
> In the slipstream we will stay
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] org-refile: Fix inconsistency when listing refile targets
2017-06-21 8:01 ` Allen Li
@ 2017-06-28 22:03 ` Allen Li
2017-07-03 6:44 ` Nicolas Goaziou
0 siblings, 1 reply; 15+ messages in thread
From: Allen Li @ 2017-06-28 22:03 UTC (permalink / raw)
To: Sebastian Reuße; +Cc: emacs-orgmode
I just realized that this patch was applied to the master branch, not
the maint branch.
The absence of this patch results in the continued life of a bug in
the maint branch where it is not possible to refile to the top level
of an agenda file when using full-file-path for
org-refile-use-outline-path.
Is it possible to backport this to the maint branch? This will
require adding the org-protect-slash that I mentioned earlier in this
thread. Otherwise, refile is basically unusable for me when
full-file-path is set.
On Wed, Jun 21, 2017 at 1:01 AM, Allen Li <vianchielfaura@gmail.com> wrote:
> Thanks for the info. I was chasing down this bug in org-20170606,
> where it still required `org-protect-slash`, so I was anxious about
> its absence in your patch.
>
> On Wed, Jun 21, 2017 at 12:55 AM, Sebastian Reuße <seb@wirrsal.net> wrote:
>> Hello Allen,
>>
>> Allen Li <vianchielfaura@gmail.com> writes:
>>
>>> On Mon, May 15, 2017 at 5:54 AM, Sebastian Reuße <seb@wirrsal.net> wrote:
>>>> * org.el (org-refile-get-targets): Setting org-refile-use-outline-path
>>>> to `file' or `buffer-name' causes an additional target for the file’s
>>>> root node to be inserted. This functionality was absent when using
>>>> `full-file-path'. We now add this since it is convenient and makes the
>>>> behavior more consistent.
>>>> ---
>>>> lisp/org.el | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/lisp/org.el b/lisp/org.el
>>>> index 97713c523..28277e352 100644
>>>> --- a/lisp/org.el
>>>> +++ b/lisp/org.el
>>>> @@ -11563,6 +11563,8 @@ (defun org-refile-get-targets (&optional default-buffer)
>>>> (push (list (file-name-nondirectory f) f nil nil) tgs))
>>>> (when (eq org-refile-use-outline-path 'buffer-name)
>>>> (push (list (buffer-name (buffer-base-buffer)) f nil nil) tgs))
>>>> + (when (eq org-refile-use-outline-path 'full-file-path)
>>>> + (push (list (file-truename (buffer-file-name (buffer-base-buffer))) f nil nil) tgs))
>>>> (org-with-wide-buffer
>>>> (goto-char (point-min))
>>>> (setq org-outline-path-cache nil)
>>>> --
>>>> 2.13.0
>>>>
>>>>
>>>
>>> If I'm not mistaken, the full file path needs to be wrapped in
>>> org-protect-slash?
>>
>> Slashes that are part of file paths aren’t escaped anymore, as of
>> 53bcf91a9. The only reason to escape slashes is to indicate they are not
>> part of the outline or file path, but part of the headline instead.
>>
>> In fact it looks like Nicolas removed ‘org-protect-slash’ in 2b44a1a74
>> and inlined it at its only use site.
>>
>> Kind regards,
>>
>> Sebastian
>>
>> --
>> Insane cobra split the wood
>> Trader of the lowland breed
>> Call a jittney, drive away
>> In the slipstream we will stay
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] org-refile: Fix inconsistency when listing refile targets
2017-06-28 22:03 ` Allen Li
@ 2017-07-03 6:44 ` Nicolas Goaziou
2017-07-07 6:55 ` Allen Li
0 siblings, 1 reply; 15+ messages in thread
From: Nicolas Goaziou @ 2017-07-03 6:44 UTC (permalink / raw)
To: Allen Li; +Cc: emacs-orgmode, Sebastian Reuße
Hello,
Allen Li <vianchielfaura@gmail.com> writes:
> I just realized that this patch was applied to the master branch, not
> the maint branch.
>
> The absence of this patch results in the continued life of a bug in
> the maint branch where it is not possible to refile to the top level
> of an agenda file when using full-file-path for
> org-refile-use-outline-path.
>
> Is it possible to backport this to the maint branch? This will
> require adding the org-protect-slash that I mentioned earlier in this
> thread. Otherwise, refile is basically unusable for me when
> full-file-path is set.
Wouldn't 2906e50177d47b4f4a0e75532a0c9dcc9cf72824 be sufficient in your
case?
If not, what commits are required?
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] org-refile: Fix inconsistency when listing refile targets
2017-07-03 6:44 ` Nicolas Goaziou
@ 2017-07-07 6:55 ` Allen Li
0 siblings, 0 replies; 15+ messages in thread
From: Allen Li @ 2017-07-07 6:55 UTC (permalink / raw)
To: Nicolas Goaziou; +Cc: emacs-orgmode, Sebastian Reuße
Yes, 2906e50177d47 is needed on maint. However, my understanding is
that on maint we still escape filesystem slashes in the outline path?
If so, org-protect-path will be needed around the file-truename in
2906e50177d4
My workflow recently changed such that I no longer need to set
org-refile-use-outline-path to full-file-path, so my interest in this
bug has dropped accordingly, but it is still a bug on maint that
breaks refiling, so I think the fix should be backported unless there
are plans for a main -> maint merge soon.
On Sun, Jul 2, 2017 at 11:44 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
> Hello,
>
> Allen Li <vianchielfaura@gmail.com> writes:
>
>> I just realized that this patch was applied to the master branch, not
>> the maint branch.
>>
>> The absence of this patch results in the continued life of a bug in
>> the maint branch where it is not possible to refile to the top level
>> of an agenda file when using full-file-path for
>> org-refile-use-outline-path.
>>
>> Is it possible to backport this to the maint branch? This will
>> require adding the org-protect-slash that I mentioned earlier in this
>> thread. Otherwise, refile is basically unusable for me when
>> full-file-path is set.
>
> Wouldn't 2906e50177d47b4f4a0e75532a0c9dcc9cf72824 be sufficient in your
> case?
>
> If not, what commits are required?
>
> Regards,
>
> --
> Nicolas Goaziou
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] Add tests for org-refile-get-targets
2017-05-15 12:54 [PATCH 1/2] Add tests for org-refile-get-targets Sebastian Reuße
2017-05-15 12:54 ` [PATCH 2/2] org-refile: Fix inconsistency when listing refile targets Sebastian Reuße
@ 2017-05-15 16:38 ` Nicolas Goaziou
2017-05-17 18:43 ` Sebastian Reuße
2017-05-17 18:44 ` [PATCH] " Sebastian Reuße
1 sibling, 2 replies; 15+ messages in thread
From: Nicolas Goaziou @ 2017-05-15 16:38 UTC (permalink / raw)
To: Sebastian Reuße; +Cc: emacs-orgmode
Hello,
Sebastian Reuße <seb@wirrsal.net> writes:
> * testing/lisp/test-org.el: Add test.
> ---
Thank you.
> testing/examples/refile/a.org | 6 ++++++
> +\f
> +;;; org-refile
Nitpick: Sections in test-org.el are sorted alphabetically. So the new
"Refile" section could go between "Radio Targets" and "Sparse trees".
> +(ert-deftest test-org/org-refile-get-targets ()
> + "Test `org-refile-get-targets'."
> + (save-window-excursion
> + (let ((examples-dir (file-truename "../examples/refile/")))
> + (cd examples-dir)
> + (find-file-read-only "a.org")
> + (find-file-read-only "b.org")
> + (rename-buffer "gratuitous-prefix/b.org")
> + (let ((org-refile-targets '((("a.org" "b.org") :level . 2)))
> + (testcases
> + `((nil . ("a/1/2"
> + "a/2/2"
> + "b/1/2"
> + "b/2/2"))
> + (file . ("a.org"
> + "a.org/a\\/1\\/1/a\\/1\\/2"
> + "a.org/a\\/2\\/1/a\\/2\\/2"
> + "b.org"
> + "b.org/b\\/1\\/1/b\\/1\\/2"
> + "b.org/b\\/2\\/1/b\\/2\\/2"))
> + (full-file-path . ,(mapcar (lambda (s) (concat examples-dir s))
> + '("a.org"
> + "a.org/a\\/1\\/1/a\\/1\\/2"
> + "a.org/a\\/2\\/1/a\\/2\\/2"
> + "b.org"
> + "b.org/b\\/1\\/1/b\\/1\\/2"
> + "b.org/b\\/2\\/1/b\\/2\\/2")))
> + (buffer-name . ("a.org"
> + "a.org/a\\/1\\/1/a\\/1\\/2"
> + "a.org/a\\/2\\/1/a\\/2\\/2"
> + "gratuitous-prefix/b.org"
> + "gratuitous-prefix/b.org/b\\/1\\/1/b\\/1\\/2"
> + "gratuitous-prefix/b.org/b\\/2\\/1/b\\/2\\/2")))))
> + (cl-loop for (use-outline-path . expected-targets) in testcases
> + do (let ((org-refile-use-outline-path use-outline-path))
> + (should
> + (equal
> + (mapcar #'car
> + (org-refile-get-targets))
> + expected-targets))))))))
> +
Would it be possible to split this big test into smaller ones, with
a description about what is really tested? See other tests in
"test-org.el" for some examples. Big tests tend to not being very
informative when they fail. IMO, code duplication is not an issue in
test files when it makes tests more readable/useful.
It would be even better if you can avoid relying on real files ("a.org"
and "b.org" in your patch), but if it makes the test too convoluted, no
worries.
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/2] Add tests for org-refile-get-targets
2017-05-15 16:38 ` [PATCH 1/2] Add tests for org-refile-get-targets Nicolas Goaziou
@ 2017-05-17 18:43 ` Sebastian Reuße
2017-05-22 0:38 ` Nicolas Goaziou
2017-05-17 18:44 ` [PATCH] " Sebastian Reuße
1 sibling, 1 reply; 15+ messages in thread
From: Sebastian Reuße @ 2017-05-17 18:43 UTC (permalink / raw)
To: Nicolas Goaziou; +Cc: emacs-orgmode
Hello Nicolas,
Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
> Nitpick: Sections in test-org.el are sorted alphabetically. So the new
> "Refile" section could go between "Radio Targets" and "Sparse trees".
Thank you, I hadn’t noticed.
> Would it be possible to split this big test into smaller ones, with
> a description about what is really tested? See other tests in
> "test-org.el" for some examples. Big tests tend to not being very
> informative when they fail. IMO, code duplication is not an issue in
> test files when it makes tests more readable/useful.
Sure. Have a look at the follow-up patch and let me know what you think.
It didn’t feel right copy-pasting the tests wholesale, so I made a
helper-macro. I checked the ert output by forcing a failure and the
failure explanation looks as expected. Does this work for you?
> It would be even better if you can avoid relying on real files ("a.org"
> and "b.org" in your patch), but if it makes the test too convoluted, no
> worries.
In the follow-up, I stuck with this primarily because of the
‘full-file-path’ test. I figure one could somehow mock a file-backed
buffer, but I expected that would be too involved just to have
everything inlined in the test. Let me know if you think differently
though.
Kind regards,
SR
--
Insane cobra split the wood
Trader of the lowland breed
Call a jittney, drive away
In the slipstream we will stay
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/2] Add tests for org-refile-get-targets
2017-05-17 18:43 ` Sebastian Reuße
@ 2017-05-22 0:38 ` Nicolas Goaziou
2017-05-22 6:46 ` Sebastian Reuße
0 siblings, 1 reply; 15+ messages in thread
From: Nicolas Goaziou @ 2017-05-22 0:38 UTC (permalink / raw)
To: Sebastian Reuße; +Cc: emacs-orgmode
Hello,
Sebastian Reuße <seb@wirrsal.net> writes:
> Sure. Have a look at the follow-up patch and let me know what you
> think.
Thank you!
> It didn’t feel right copy-pasting the tests wholesale, so I made a
> helper-macro. I checked the ert output by forcing a failure and the
> failure explanation looks as expected. Does this work for you?
Honestly, I still find it difficult to follow and debug when one the
tests is failing. Also, we can't use `alist-get' since we are still
supporting Emacs 24.3.
Taking inspiration from your patch, I wrote a test for
`refile-get-targets' in f335c3517de06eb74a3c3727843f276147795a84. It has
more code duplication than yours. OTOH, it doesn't require any
hard-coded file. Hopefully, it will be simpler to debug when a problem
arises in the function.
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] Add tests for org-refile-get-targets
2017-05-22 0:38 ` Nicolas Goaziou
@ 2017-05-22 6:46 ` Sebastian Reuße
2017-05-22 6:55 ` Nicolas Goaziou
0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Reuße @ 2017-05-22 6:46 UTC (permalink / raw)
To: Nicolas Goaziou; +Cc: emacs-orgmode
Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
> Sebastian Reuße <seb@wirrsal.net> writes:
>> It didn’t feel right copy-pasting the tests wholesale, so I made a
>> helper-macro. I checked the ert output by forcing a failure and the
>> failure explanation looks as expected. Does this work for you?
> Honestly, I still find it difficult to follow and debug when one the
> tests is failing. Also, we can't use `alist-get' since we are still
> supporting Emacs 24.3.
Fair enough. Thank you! FWIW, is there some handy table to consult on
newly supported functions by Emacs version? Or do you have it all
memorized from the Emacs changelogs?
Kind regards,
--
Insane cobra split the wood
Trader of the lowland breed
Call a jittney, drive away
In the slipstream we will stay
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] Add tests for org-refile-get-targets
2017-05-22 6:46 ` Sebastian Reuße
@ 2017-05-22 6:55 ` Nicolas Goaziou
0 siblings, 0 replies; 15+ messages in thread
From: Nicolas Goaziou @ 2017-05-22 6:55 UTC (permalink / raw)
To: Sebastian Reuße; +Cc: emacs-orgmode
Hello,
Sebastian Reuße <seb@wirrsal.net> writes:
> Fair enough. Thank you! FWIW, is there some handy table to consult on
> newly supported functions by Emacs version? Or do you have it all
> memorized from the Emacs changelogs?
When I encounter a suspicious function, i.e., a function I have hardly
seen before, I use <F1 N> an search for it in the document. I also
quickly browse Emacs 24.4 and 24.5 changes with <C-u F1 N>.
That's rather low-level.
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] Add tests for org-refile-get-targets
2017-05-15 16:38 ` [PATCH 1/2] Add tests for org-refile-get-targets Nicolas Goaziou
2017-05-17 18:43 ` Sebastian Reuße
@ 2017-05-17 18:44 ` Sebastian Reuße
1 sibling, 0 replies; 15+ messages in thread
From: Sebastian Reuße @ 2017-05-17 18:44 UTC (permalink / raw)
To: emacs-orgmode; +Cc: Sebastian Reuße
* testing/lisp/test-org.el: Add test.
---
testing/examples/refile/a.org | 6 ++++
testing/examples/refile/b.org | 6 ++++
testing/lisp/test-org.el | 71 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+)
create mode 100644 testing/examples/refile/a.org
create mode 100644 testing/examples/refile/b.org
diff --git a/testing/examples/refile/a.org b/testing/examples/refile/a.org
new file mode 100644
index 000000000..7ed235a52
--- /dev/null
+++ b/testing/examples/refile/a.org
@@ -0,0 +1,6 @@
+* a/1/1
+** a/1/2
+*** a/1/3
+* a/2/1
+** a/2/2
+*** a/2/3
diff --git a/testing/examples/refile/b.org b/testing/examples/refile/b.org
new file mode 100644
index 000000000..15359b16e
--- /dev/null
+++ b/testing/examples/refile/b.org
@@ -0,0 +1,6 @@
+* b/1/1
+** b/1/2
+*** b/1/3
+* b/2/1
+** b/2/2
+*** b/2/3
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index b8bd88957..099817064 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -5266,6 +5266,77 @@ (ert-deftest test-org/update-radio-target-regexp ()
(org-element-type (org-element-context))))))
\f
+;;; Refile
+
+(defvar test-org/refile-use-outline-path-examples-dir
+ (file-truename "../examples/refile/")
+ "Absolute path to example files used in ‘org-refile’ tests.")
+
+(defvar test-org/refile-use-outline-path-targets
+ `((nil . ("a/1/2"
+ "a/2/2"
+ "b/1/2"
+ "b/2/2"))
+ (file . ("a.org"
+ "a.org/a\\/1\\/1/a\\/1\\/2"
+ "a.org/a\\/2\\/1/a\\/2\\/2"
+ "b.org"
+ "b.org/b\\/1\\/1/b\\/1\\/2"
+ "b.org/b\\/2\\/1/b\\/2\\/2"))
+ (full-file-path . ,(mapcar (lambda (s)
+ (concat test-org/refile-use-outline-path-examples-dir s))
+ '("a.org"
+ "a.org/a\\/1\\/1/a\\/1\\/2"
+ "a.org/a\\/2\\/1/a\\/2\\/2"
+ "b.org"
+ "b.org/b\\/1\\/1/b\\/1\\/2"
+ "b.org/b\\/2\\/1/b\\/2\\/2")))
+ (buffer-name . ("a.org"
+ "a.org/a\\/1\\/1/a\\/1\\/2"
+ "a.org/a\\/2\\/1/a\\/2\\/2"
+ "gratuitous-prefix/b.org"
+ "gratuitous-prefix/b.org/b\\/1\\/1/b\\/1\\/2"
+ "gratuitous-prefix/b.org/b\\/2\\/1/b\\/2\\/2")))
+ "Alist of ‘refile-use-outline-path’ settings and associated \
+expected refile targets.")
+
+(defmacro test-org/refile-get-targets (use-outline-path)
+ "Helper macro to test ‘org-refile-get-targets’ with \
+‘org-refile-use-outline-path’ set to USE-OUTLINE-PATH."
+ `(save-window-excursion
+ (let ((org-refile-targets '((("a.org" "b.org")
+ :level . 2)))
+ (org-refile-use-outline-path ,use-outline-path))
+ (cd test-org/refile-use-outline-path-examples-dir)
+ (find-file-read-only "a.org")
+ (find-file-read-only "b.org")
+ (rename-buffer "gratuitous-prefix/b.org")
+ (should (equal
+ (mapcar #'car (org-refile-get-targets))
+ (alist-get ,use-outline-path
+ test-org/refile-use-outline-path-targets))))))
+
+(ert-deftest test-org/refile-get-targets-no-outline-path ()
+ "Test ‘org-refile-get-targets’ with \
+‘org-refile-use-outline-path’ to ‘nil’."
+ (test-org/refile-get-targets nil))
+
+(ert-deftest test-org/refile-get-targets-full-file-path ()
+ "Test ‘org-refile-get-targets’ with \
+‘org-refile-use-outline-path’ set to ‘full-file-path’."
+ (test-org/refile-get-targets 'full-file-path))
+
+(ert-deftest test-org/refile-get-targets-buffer-name ()
+ "Test `org-refile-get-targets' with \
+`org-refile-use-outline-path' set to ‘buffer-name’."
+ (test-org/refile-get-targets 'buffer-name))
+
+(ert-deftest test-org/refile-get-targets-file ()
+ "Test `org-refile-get-targets' with \
+`org-refile-use-outline-path' set to ‘file’."
+ (test-org/refile-get-targets 'file))
+
+\f
;;; Sparse trees
(ert-deftest test-org/match-sparse-tree ()
--
2.13.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-07-07 6:55 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-15 12:54 [PATCH 1/2] Add tests for org-refile-get-targets Sebastian Reuße
2017-05-15 12:54 ` [PATCH 2/2] org-refile: Fix inconsistency when listing refile targets Sebastian Reuße
2017-05-17 12:54 ` Nicolas Goaziou
2017-06-21 7:05 ` Allen Li
2017-06-21 7:55 ` Sebastian Reuße
2017-06-21 8:01 ` Allen Li
2017-06-28 22:03 ` Allen Li
2017-07-03 6:44 ` Nicolas Goaziou
2017-07-07 6:55 ` Allen Li
2017-05-15 16:38 ` [PATCH 1/2] Add tests for org-refile-get-targets Nicolas Goaziou
2017-05-17 18:43 ` Sebastian Reuße
2017-05-22 0:38 ` Nicolas Goaziou
2017-05-22 6:46 ` Sebastian Reuße
2017-05-22 6:55 ` Nicolas Goaziou
2017-05-17 18:44 ` [PATCH] " Sebastian Reuße
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).