From: Brian Carlson <hacker@abutilize.com>
To: emacs-orgmode@gnu.org
Subject: Re: Starting source code export at non-zero (-n value)
Date: Sun, 22 May 2016 11:57:05 -0400 [thread overview]
Message-ID: <5741D6D1.6030903@abutilize.com> (raw)
In-Reply-To: <87k2iobglr.fsf@saiph.selenimh>
On 2016-05-20 16:48, Nicolas Goaziou wrote:
> Hello,
>
> Brian Carlson <hacker@abutilize.com> 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 <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.
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
next prev parent reply other threads:[~2016-05-22 15:57 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
2016-05-22 15:57 ` Brian Carlson [this message]
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=5741D6D1.6030903@abutilize.com \
--to=hacker@abutilize.com \
--cc=emacs-orgmode@gnu.org \
/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).