From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [PATCH] org-table-beginning/end-of-field Date: Fri, 12 Sep 2014 10:29:48 +0200 Message-ID: <8738bxqm0z.fsf@nicolasgoaziou.fr> References: <87mwaaekp8.fsf@sophokles.streitblatt.de> <871trmbe92.fsf@nicolasgoaziou.fr> <87egvme27x.fsf_-_@sophokles.streitblatt.de> <87wq9e9lkn.fsf@nicolasgoaziou.fr> <87ppf59hjs.fsf@sophokles.streitblatt.de> <87oauna8xm.fsf@nicolasgoaziou.fr> <871trjo13j.fsf@sophokles.streitblatt.de> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:54100) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSMEG-0003Dc-MN for emacs-orgmode@gnu.org; Fri, 12 Sep 2014 04:29:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XSME8-0007IM-Iq for emacs-orgmode@gnu.org; Fri, 12 Sep 2014 04:29:16 -0400 Received: from relay6-d.mail.gandi.net ([2001:4b98:c:538::198]:33255) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSME8-0007IG-8X for emacs-orgmode@gnu.org; Fri, 12 Sep 2014 04:29:08 -0400 In-Reply-To: <871trjo13j.fsf@sophokles.streitblatt.de> (Florian Beck's message of "Wed, 10 Sep 2014 19:08:00 +0200") 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: Florian Beck Cc: emacs-orgmode@gnu.org Hello, Florian Beck writes: Thanks for your feedback. > This I did. Well, (or (org-element-get 'table-cell) ;; Fuzzy matching when on a table border: (org-element-get 'table-cell (1+ (point))) (org-element-get 'table-cell (1- (point)))) is not necessary, since the first object/element returned should give you enough information. Also, I forgot the following case: | a | b | |---+-X-| | c | d | I think a correct analysis of the current object can solve all tricky cases, without blindly poking around. >> X | a | b | >> | c | d | >> >> | a | b | X >> | c | d | >> >> | a | b | >> X | c | d | > > This is arguably outside the scope of org-table-beginning/end, isn't it? The current implementation handles them (sometimes incorrectly). I find this sloppiness useful. > Come to think of it, these functions should only move to the > beginning or end of the field, period. Then handle the special case in > org-forward/backward-sentence or create two new functions. OTOH, is it really useful to isolate beginning/end-of-field feature? I'm not sure, but feel free to try if you think it can make the code better. >> (defun org-element-parent (blob &optional types genealogy) >> "Return BLOB's parent, or nil. >> >> BLOB is an object or element. >> >> When optional argument TYPES is a list of symbols, return the >> first element or object between BLOB and its ancestors whose type >> belongs to that list. >> >> When optional argument GENEALOGY is non-nil, return a list of all >> ancestors, from closest to farthest. >> >> When BLOB is obtained through `org-element-context' or >> `org-element-at-point', only ancestors from its section can be >> found. There is no such limitation when BLOB belongs to a full >> parse tree." >> (if (not (or types genealogy)) (org-element-property :parent blob) >> (let ((up blob) ancestors) >> (while (and up (not (memq (org-element-type up) types))) >> (when genealogy (push up ancestors)) >> (setq up (org-element-property :parent up))) >> (if genealogy (nreverse ancestors) up)))) >> >> Its name is a bit confusing since it can return BLOB under some optional >> argument combination (i.e., if BLOB's type belongs to TYPES). However, >> it covers most, if not all, needs about ancestors. WDYT? > > Yes this works. Making TYPES into a list is a nice touch. However: > - the advertised functionality (returning the parent) is only > marginally useful and handled in half a line of code; whereas most of > the functionality is hidden as optional; That's my intention. The functionality is very common so there is a high chance that it will be remembered. It is possible to separate the features and create, let's say, `org-element-genealogy'. However, in this case, I lean towards overloading a common function instead of creating a dedicated niche one. > - this leads to confusing behaviour, as you have noted, because > the type of object I'm looking for, is not necessarily the parent; This is why it can return the current object when TYPES is provided. If we create another function, you will need to check both the current item and the return value of that function. > - BLOB on the other hand should be optional, because defaulting it to > org-element-context would be highly useful. I don't consider "highly useful" the fact that you could call (org-element-parent) instead of (org-element-parent (org-element-context)) However I see two drawbacks in this optional argument: it hides the call to most expensive function and `org-element-context' is not always what you want (there's no reason to prefer it over `org-element-at-point' and the other way around). Eventually, I'd like to keep `org-element-at-point', `org-element-context' and `org-element-parse-buffer' as the highest level functions in "org-element.el" (i.e., no other function should call them there). Using these functions is out of the scope of the parser. > - not sure what GENEALOGY is useful for; You may grep for `org-export-get-genealogy'. There are a couple of places where this one is used. Actually, this function would also generalize: - `org-export-get-parent-headline' - `org-export-get-parent-element' - `org-export-get-parent-table' > if the user also specified TYPES, this returns the ancestors up to but > NOT including those of TYPE. I admit I didn't put more thoughts into TYPES + GENEALOGY combination. Of course, if this function is implemented, I agree it should also include the first element matching TYPES. > That's another possibility. Unlike a link without description, one might > argue that an empty cell has a natural contents position. For the time being, I prefer all parsing functions to be consistent. Regards, -- Nicolas Goaziou