emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Brian Carlson <hacker@abutilize.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: Starting source code export at non-zero (-n value)
Date: Fri, 20 May 2016 22:48:48 +0200	[thread overview]
Message-ID: <87k2iobglr.fsf@saiph.selenimh> (raw)
In-Reply-To: <573A9100.3020503@abutilize.com> (Brian Carlson's message of "Mon, 16 May 2016 23:33:20 -0400")

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

  reply	other threads:[~2016-05-20 22:42 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-17  3:33 Starting source code export at non-zero (-n value) Brian Carlson
2016-05-20 20:48 ` Nicolas Goaziou [this message]
2016-05-22 15:57   ` Brian Carlson
2016-05-23  4:00   ` PATCH: ox: " Brian Carlson
2016-05-24 19:44     ` Nicolas Goaziou
2016-05-24 20:33       ` Nicolas Goaziou
2016-05-26  3:06         ` Brian Carlson
2016-05-26  6:52           ` Nicolas Goaziou
2016-05-30  2:42             ` Brian Carlson
2016-05-30  2:45             ` Brian Carlson
2016-05-31 16:47             ` Brian Carlson
2016-05-31 20:37               ` Nicolas Goaziou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k2iobglr.fsf@saiph.selenimh \
    --to=mail@nicolasgoaziou.fr \
    --cc=emacs-orgmode@gnu.org \
    --cc=hacker@abutilize.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).