emacs-orgmode
[Top][All Lists]
Advanced

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

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


From: Nicolas Goaziou
Subject: Re: [O] Starting source code export at non-zero (-n value)
Date: Fri, 20 May 2016 22:48:48 +0200

Hello,

Brian Carlson <address@hidden> 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 <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.

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



reply via email to

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