Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-18 Thread Bastien
Bastien b...@gnu.org writes: Ilya Shlyakhter ilya_...@alum.mit.edu writes: When I open emacs with this file, move to the emacs-lisp block, and evaluate it, I get aaa. I can reproduce your problem now, I'm on it, and the problem is real, but I need to make sure all tests pass fine before

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-18 Thread Ilya Shlyakhter
Thanks Bastien. Property API documentation could be made more precise in some places. From the documentation of org-entry-get (both the docstring and the Org manual), it would seem that unless the inherit argument is non-nil, file-wide and system-wide property settings should not be checked at

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-18 Thread Achim Gratz
Bastien writes: Okay, so I committed a different fix in maint and master. That fix (and the explanation) makes much more sense… I'll have to see that this also gets tested, but I won't get to it for some time, unfortunately. Regards, Achim. -- +[Q+ Matrix-12 WAVE#46+305 Neuron microQkb

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-18 Thread Bastien
Hi Achim, Achim Gratz strom...@nexgo.de writes: Bastien writes: Okay, so I committed a different fix in maint and master. That fix (and the explanation) makes much more sense… I'll have to see that this also gets tested, but I won't get to it for some time, unfortunately. I'll first fix

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-17 Thread Achim Gratz
Bastien writes: Applied, thanks! That badly breaks the following tests: FAILED test-ob-header-arg-defaults/tree/accumulate/call FAILED test-ob-header-arg-defaults/tree/accumulate/noweb FAILED test-ob-header-arg-defaults/tree/complex/call FAILED

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-17 Thread Bastien
Achim Gratz strom...@nexgo.de writes: Bastien writes: Applied, thanks! That badly breaks the following tests: FAILED test-ob-header-arg-defaults/tree/accumulate/call FAILED test-ob-header-arg-defaults/tree/accumulate/noweb FAILED test-ob-header-arg-defaults/tree/complex/call

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-17 Thread Achim Gratz
Bastien writes: Can you tell a bit more about what's wrong with the test? There is nothing wrong with those tests. If the patch is good and the tests are outdated, I'd rather fix the tests than revert the patch to re-revert it again. No, the patch is bad, otherwise it wouldn't break the

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-17 Thread Bastien
Achim Gratz strom...@nexgo.de writes: Can you tell a bit more about what's wrong with the test? There is nothing wrong with those tests. I meant: can you tell me how the tests fail? I'm interested in the answer. If the patch is good and the tests are outdated, I'd rather fix the tests

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-17 Thread Achim Gratz
Bastien writes: Achim Gratz strom...@nexgo.de writes: Can you tell a bit more about what's wrong with the test? There is nothing wrong with those tests. I meant: can you tell me how the tests fail? They don't produce the result they are supposed to produce. I'm interested in the answer.

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-17 Thread Bastien
Achim Gratz strom...@nexgo.de writes: I meant: can you tell me how the tests fail? They don't produce the result they are supposed to produce. Thanks for this explanation. I'm interested in the answer. make BTEST_RE='\\(header-arg-defaults\\|property-accumulation\\)' test-dirty Thanks!

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-17 Thread Achim Gratz
Bastien writes: What I meant is this: broken tests are not a sufficient reason to revert a commit. You need to show the commit is wrong and the tests are not outdated. No code breaking a test should have been committed in the first place, then we wouldn't need to have this discussion. If the

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-17 Thread Bastien
Achim Gratz strom...@nexgo.de writes: Bastien writes: What I meant is this: broken tests are not a sufficient reason to revert a commit. You need to show the commit is wrong and the tests are not outdated. No code breaking a test should have been committed in the first place, then we

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-17 Thread Achim Gratz
Ilya Shlyakhter writes: Here is the test case again: The test case doesn't work as posted. A working test case produces the result shown below (with and without your patch reverted) on current master (tested again via make vanilla just to be sure). --8---cut

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-17 Thread Ilya Shlyakhter
In the current master branch, doing the example from the patch (reproduced below again) gives aaa, because the line (let (org-file-properties org-global-properties org-global-properties-fixed) has been removed from org-entry-get-with-inheritance . I agree that patching a function as core

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-17 Thread Ilya Shlyakhter
On 3/17/14 5:43 PM, Achim Gratz wrote: The test case doesn't work as posted. A working test case produces the Try http://ilya.cc/testcase.org When I open emacs with this file, move to the emacs-lisp block, and evaluate it, I get aaa. I could easily be wrong re: the logic of the code, but

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-17 Thread Bastien
Ilya Shlyakhter ilya_...@alum.mit.edu writes: When I open emacs with this file, move to the emacs-lisp block, and evaluate it, I get aaa. I can reproduce your problem now, I'm on it, and the problem is real, but I need to make sure all tests pass fine before fixing this. Thanks, -- Bastien

Re: [O] [PATCH] Fixed bug in org-entry-get-with-inheritance

2014-03-14 Thread Bastien
Applied, thanks! -- Bastien