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

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Hello,

Brian Carlson <hacker@abutilize.com> writes:

> Hello other org mode aficionados! (I apologize for the errant email I 
> send a minute ago. )
>
> I have created a (possible) patch to the export (ox.el and other 
> ox-XXX.el) files that allow for "source code" and "example" blocks to 
> have line numbers starting at an arbitrary number. Before the 
> (wonderful) ox redesign I used to have advice around some functions. I 
> hadn't needed this functionality for a while, but now that I did I 
> thought I would share.
>
>
> The code is written with the following design:
>
> -n   is the same as -n 1 : The functionality is unchanged
> +n   is the same as +n 1 :  The functionality is unchanged
>
> -n X   will "reset" and start new code block starting at line X
> +n X   will "add" X to the last line of the block before.

Thank you for the patch. It looks like an useful addition to Org. Some
comments follow.

> From 6b4db0a978cc3492f0d0ac7e29008de6846fbe4a Mon Sep 17 00:00:00 2001
> From: Brian Carlson <hacker@abutilize.com>
> 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.

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

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.

> -			((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 'new 0))
> +			((string-match "+n" switches) (cons 'continued 0))
> +			))

These parens should be moved at the end of the previous line.

>  		 (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]".

>  			     "\\(.*\\)[ \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.

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

WDYT?


Regards,

-- 
Nicolas Goaziou