From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brian Carlson Subject: Re: Starting source code export at non-zero (-n value) Date: Sun, 22 May 2016 11:57:05 -0400 Message-ID: <5741D6D1.6030903@abutilize.com> References: <573A9100.3020503@abutilize.com> <87k2iobglr.fsf@saiph.selenimh> Reply-To: hacker@abutilize.com Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:45617) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4Vks-0003FL-8i for emacs-orgmode@gnu.org; Sun, 22 May 2016 11:57:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b4Vko-0006hm-1y for emacs-orgmode@gnu.org; Sun, 22 May 2016 11:57:25 -0400 Received: from gateway24.websitewelcome.com ([192.185.51.59]:54792) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4Vkn-0006h4-P0 for emacs-orgmode@gnu.org; Sun, 22 May 2016 11:57:21 -0400 Received: from cm7.websitewelcome.com (cm7.websitewelcome.com [108.167.139.20]) by gateway24.websitewelcome.com (Postfix) with ESMTP id DEE1B662319BD for ; Sun, 22 May 2016 10:57:06 -0500 (CDT) Received: from pool-173-76-187-225.bstnma.fios.verizon.net ([173.76.187.225]:46404 helo=[192.168.1.11]) by gator3208.hostgator.com with esmtpsa (TLSv1.2:DHE-RSA-AES128-SHA:128) (Exim 4.86_1) (envelope-from ) id 1b4VkW-000ON2-Td for emacs-orgmode@gnu.org; Sun, 22 May 2016 10:57:04 -0500 In-Reply-To: <87k2iobglr.fsf@saiph.selenimh> 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" To: emacs-orgmode@gnu.org On 2016-05-20 16:48, Nicolas Goaziou wrote: > Hello, > > Brian Carlson writes: > >> Hello other org mode aficionados! (I apologize for the errant email I >> send a minute ago. ) > >> From 6b4db0a978cc3492f0d0ac7e29008de6846fbe4a Mon Sep 17 00:00:00 2001 >> From: Brian Carlson >> Date: Mon, 16 May 2016 10:58:01 -0400 >> Subject: [PATCH] ox: provide [+-]n functionality > > Here you need to specify all the functions being modified. You may want > to look at other commit messages in the repository. I'll make the appropriate commentary in my commit message in my branch. > >> @@ -1488,9 +1488,9 @@ contextual information." >> (custom-env (and lang >> (cadr (assq (intern lang) >> org-groff-custom-lang-environments)))) >> - (num-start (case (org-element-property :number-lines src-block) >> + (num-start (case (car (org-element-property :number-lines src-block)) >> (continued (org-export-get-loc src-block info)) >> - (new 0))) >> + (new (or (cdr (org-element-property :number-lines src-block)) 0)))) > > This pattern appears often in your patch, and, of course in the code > base. I suggest to factorize it out. Indeed, we could take advantage of > the new behaviour of `org-export-get-loc' that your patch introduces. I started doing that, and with your recommendation. I'll do just that. > > IIUC, the new `org-export-get-loc' returns the number for the first line > of the current block, not the number of the last line in the previous > block, like it used to do. It can then includes the construct above: > > (pcase (org-element-property :number-lines src-block) > ;; No need to compute line numbers before this one. > (`(new . ,n) (or n 2)) > (`(continued . ,_) > ;; Count all lines above, up to the first block that has "new" line > ;; numbering. > (let ((loc 0)) > (org-element-map (plist-get info :parse-tree) > ...)))) > > Of course, tests would have to be changed or updated accordingly. A very nice recommendation. I'll look into this. > >> - ((string-match "-n\\>" switches) 'new) >> - ((string-match "+n\\>" switches) 'continued))) >> + ((string-match "-n *\\([0-9]+\\)" switches) (cons 'new (- (string-to-number (match-string 1 switches)) 1 ))) > > There is a spurious white space after the last 1. >> + ((string-match "+n *\\([0-9]+\\)" switches) (cons 'continued (- (string-to-number (match-string 1 switches)) 1 ))) > > Ditto. >> + ((string-match "+n" switches) (cons 'continued 0)) >> + )) > > These parens should be moved at the end of the previous line. Nice catch. Before I resubmit the patch. I'll clean up this and the other non-standard coding conventions. I haven't submitted patches to emacs code before, so I really do appreciate bringing my attention to areas where I fail to follow the coding conventions. > >> (preserve-indent >> (and switches (string-match "-i\\>" switches))) >> ;; Should labels be retained in (or stripped from) example >> @@ -2393,7 +2396,7 @@ Assume point is at the beginning of the block." >> (looking-at >> (concat "^[ \t]*#\\+BEGIN_SRC" >> "\\(?: +\\(\\S-+\\)\\)?" >> - "\\(\\(?: +\\(?:-l \".*?\"\\|[-+][A-Za-z]\\)\\)+\\)?" >> + "\\(\\(?: +\\(?:-l \".+\"\\|[+-]n *[[:digit:]]+\\|[+-][[:alpha:]]\\)\\)+\\)?" > > I don't think "[-+][[:alpha:]]" is correct here and neither was [-+][A-Za-z]. We > only want to match valid switches: k, r and i. Besides, there is no +k, > +r or +i. Thus, it should be "-[iIkKrR]". I agree with this. I'll fix this, as well. I think we actually need -[iIkKrRnN]|+[nN] The first part of the regular expression catches [-+][Nn] *[0-9]+ but not [-+][Nn] in isolation (which obviously we still need). >> "\\(.*\\)[ \t]*$")) >> (org-match-string-no-properties 1))) >> ;; Get switches. >> @@ -2403,8 +2406,11 @@ Assume point is at the beginning of the block." >> ;; Switches analysis >> (number-lines >> (cond ((not switches) nil) >> - ((string-match "-n\\>" switches) 'new) >> - ((string-match "+n\\>" switches) 'continued))) >> + ((string-match "-n *\\([0-9]+\\)" switches) (cons 'new (- (string-to-number (match-string 1 switches)) 1 ))) >> + ((string-match "+n *\\([0-9]+\\)" switches) (cons 'continued (- (string-to-number (match-string 1 switches)) 1 ))) >> + ((string-match "-n" switches) (cons 'new 0)) >> + ((string-match "+n" switches) (cons 'continued 0)) >> + )) > > See above. I'll do some testing and tinkering. The "problem" is that -n is equal to -n 1, and not -n 0. The cons cell actually holds ('type . (- starting_number 1)). This may not be ideal, but seemed the simplest. There's probably a very simple lisp expression that can reduce the cond back to a 3 conditional clause to get what I need. (If "see above" was in regards to the regular expressions comment). Unless you were only talking about the extra space, then that's a really easy fix. ;) >> diff --git a/lisp/ox.el b/lisp/ox.el >> index 5ad17ec..c05f55e 100644 >> --- a/lisp/ox.el >> +++ b/lisp/ox.el >> @@ -4148,9 +4148,9 @@ error if no block contains REF." >> (when (re-search-backward ref-re nil t) >> (cond >> ((org-element-property :use-labels el) ref) >> - ((eq (org-element-property :number-lines el) 'continued) >> + ((eq (car (org-element-property :number-lines el)) 'continued) >> (+ (org-export-get-loc el info) (line-number-at-pos))) >> - (t (line-number-at-pos))))))) >> + (t (+ (or (cdr (org-element-property :number-lines el)) 0) (line-number-at-pos)))))))) > > Here, the last two branches of the cond could be replaced with a single > one calling the new `org-export-get-loc'. Okay. > WDYT? Very Helpful. Thanks for the direction. I'll make the changes/re-write to fix these issues and re-submit a patch. Because this goes past (or might go past) the 15-line FSF limitation for a "patch." I've got the paperwork from the FSF for copyright assignment and will be submitting this next week. (Even if a patch would fit into the limit, having the paperwork on file won't hurt). Thanks, ;-b