From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ilya Shlyakhter Subject: Re: [PATCH] Fixed bug in org-entry-get-with-inheritance Date: Mon, 17 Mar 2014 17:30:31 -0400 Message-ID: References: <87bnx9qbcq.fsf@bzg.ath.cx> <87vbvcptqt.fsf@Rainer.invalid> <87ob14d4o7.fsf@bzg.ath.cx> <87r460pr8d.fsf@Rainer.invalid> <87ha6wd3hg.fsf@bzg.ath.cx> <87mwgoppvf.fsf@Rainer.invalid> <87mwgopnrf.fsf@bzg.ath.cx> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:43451) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPf7X-0001tt-Qm for emacs-orgmode@gnu.org; Mon, 17 Mar 2014 17:30:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WPf7U-0003eB-US for emacs-orgmode@gnu.org; Mon, 17 Mar 2014 17:30:55 -0400 In-Reply-To: <87mwgopnrf.fsf@bzg.ath.cx> List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Bastien Cc: Achim Gratz , emacs-orgmode 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 as this should be done with care; however, I'm pretty positive that there was a bug, as explained in the patch message. org-entry-get-with-inheritance calls org-entry-get for each entry going up the tree, to read the property at that entry _without_ inheritance; however, unless the let line above is included, this reading actually happens _with_ inheritance -- of global properties. So a property can appear to be set at a node during up-the-tree traversal, when in fact it is only set as a global property. If org-entry-get-with-inheritance see a property set at a node during up-the-tree traversal, it stops the traversal right there, ignoring any settings of this property further up the tree -- which should override any global settings of the same property. Here is the test case again: #+PROPERTY: myprop aaa * headline A :PROPERTIES: :myprop: bbb :END: *** headline B :PROPERTIES: :otherprop: ccc :END: #+BEGIN_SRC emacs-lisp (message (org-entry-get-with-inheritance "myprop")) #} #+RESULTS: : aaa On Mon, Mar 17, 2014 at 4:35 PM, Bastien wrote: > Achim Gratz 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! > >>>>> 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 tests. >>> >>> Sorry, I don't buy this. >> >> I'm not selling anything. > > 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. > > In this case, I made the error of reproduce Ilya's solution, > not Ilya's problem, so I wrong assumed his patch was the problem > to his problem. > > Ilya: from the maint and master branch, I get "bbb" as a result > for the example you placed in your commit message. Do you have > "aaa" as a result with Org from maint or master? If so, can you > provide a recipe? > > Thanks, > > -- > Bastien