[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [O] Starting source code export at non-zero (-n value)

From: Brian Carlson
Subject: Re: [O] Starting source code export at non-zero (-n value)
Date: Sun, 22 May 2016 11:57:05 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0

On 2016-05-20 16:48, Nicolas Goaziou wrote:

Brian Carlson <address@hidden> 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 <address@hidden>
Date: Mon, 16 May 2016 10:58:01 -0400
Subject: [PATCH] ox: provide [+-]n <offset> 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)
-         (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 )))

+                       ((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.

                  (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."
                     (concat "^[ \t]*#\\+BEGIN_SRC"
                             "\\(?: +\\(\\S-+\\)\\)?"
-                            "\\(\\(?: +\\(?:-l 
+                            "\\(\\(?: +\\(?:-l \".+\"\\|[+-]n 

I don't think "[-+][[:alpha:]]" is correct here and neither was [-+][A-Za-z].  
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
                  (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)
                 ((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) 

Here, the last two branches of the cond could be replaced with a single
one calling the new `org-export-get-loc'.

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).


reply via email to

[Prev in Thread] Current Thread [Next in Thread]