emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Jumping from source block to Org block ...
@ 2013-09-14  3:04 aditya siram
  2013-09-14 15:44 ` Eric Schulte
  0 siblings, 1 reply; 6+ messages in thread
From: aditya siram @ 2013-09-14  3:04 UTC (permalink / raw)
  To: emacs-orgmode


[-- Attachment #1.1: Type: text/plain, Size: 251 bytes --]

Attached is a patch that fixes a bug with jumping from source block back to
the Org file. The problem is that the current detangling behavior does not
take the :padlline flag into account. This stopped.

Hopefully this is helpful to others ...
-deech

[-- Attachment #1.2: Type: text/html, Size: 311 bytes --]

[-- Attachment #2: src_block_jump_fix.patch --]
[-- Type: application/octet-stream, Size: 6245 bytes --]

From 1bff94700991197d236af89bcfe5fa246941c7b7 Mon Sep 17 00:00:00 2001
From: Aditya Siram <aditya siram at gmail dot com>
Date: Fri, 13 Sep 2013 18:29:14 -0500
Subject: [PATCH 3/3] org-babel-detangle and org-babel-tangle-jump-to-org now
 correctly compensate for padded source chunks

---
 lisp/ob-tangle.el | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 86 insertions(+), 6 deletions(-)

diff --git a/lisp/ob-tangle.el b/lisp/ob-tangle.el
index 8141943..d8cede2 100644
--- a/lisp/ob-tangle.el
+++ b/lisp/ob-tangle.el
@@ -502,12 +502,12 @@ which enable the original code blocks to be found."
         (goto-char end))
       (prog1 counter (message "Detangled %d code blocks" counter)))))
 
-(defun org-babel-tangle-jump-to-org ()
+(defun org-babel-tangle-jump-to-org (&optional maintain-point)
   "Jump from a tangled code file to the related Org-mode file."
   (interactive)
   (let ((mid (point))
-	start body-start end done
-        target-buffer target-char link path block-name body)
+	start body-start end done depadded-body
+        target-buffer offset target-char link path block-name body)
     (save-window-excursion
       (save-excursion
 	(while (and (re-search-backward org-bracket-link-analytic-regexp nil t)
@@ -526,7 +526,7 @@ which enable the original code blocks to be found."
 			      (setq end (point-at-bol))))))))
 	(unless (and start (< start mid) (< mid end))
 	  (error "Not in tangled code"))
-        (setq body (org-babel-trim (buffer-substring start end))))
+        (setq body (buffer-substring start end)))
       (when (string-match "::" path)
         (setq path (substring path 0 (match-beginning 0))))
       (find-file path) (setq target-buffer (current-buffer))
@@ -537,12 +537,17 @@ which enable the original code blocks to be found."
         (org-babel-goto-named-src-block block-name))
       ;; position at the beginning of the code block body
       (goto-char (org-babel-where-is-src-block-head))
+      (let* ((pad-adjusted-values (org-babel-detangle-adjust-for-padlines start mid body))
+	     (depadded-body (car pad-adjusted-values))
+	     (depadded-point (cdr pad-adjusted-values)))
+	(progn
+	  (setq offset depadded-point)
+	  (setq body depadded-body)))
       (forward-line 1)
-      ;; Use org-edit-special to isolate the code.
       (org-edit-special)
       ;; Then move forward the correct number of characters in the
       ;; code buffer.
-      (forward-char (- mid body-start))
+      (forward-char offset)
       ;; And return to the Org-mode buffer with the point in the right
       ;; place.
       (org-edit-src-exit)
@@ -550,6 +555,81 @@ which enable the original code blocks to be found."
     (org-src-switch-to-buffer target-buffer t)
     (prog1 body (goto-char target-char))))
 
+(defun org-babel-detangle-adjust-for-padlines (chunk-start desired-point chunk-body)
+  "Check if :padline was enabled for this chunk and then
+adjust point and body accordingly. Also account for the user
+editing the tangled file and removing the padded lines. 
+
+Returns a tuple (body . offset) where body is an adjusted source chunk
+with padded newlines possibly removed and offset is how far into
+that chunk point is. If point was on either of the padlines, point
+is set to the first or last character of the chunk depending on what's
+closer."
+  (save-excursion
+    (let* ((org-block (org-babel-get-src-block-info))
+	   (params (nth 2 org-block))
+	   (org-body (nth 1 org-block))
+	   (should-be-padded (not (string= "no" (cdr (assoc :padline params)))))
+	   (minimal-padded-chunk-length (+ 1 ;; end of first delimiter
+					   1 ;; newline from first padline
+					   1 ;; newline from second padline
+					   1 ;; start of second delimiter
+					   ))
+	   (possibly-padded (>= (length chunk-body) minimal-padded-chunk-length))
+	   (char-at (lambda (pos str) (substring str pos (+ pos 1))))
+	   (actually-padded  (and (not (=  (length chunk-body) 0))
+				  (string= "\n" (funcall char-at 1 chunk-body))
+				  (string= "\n" (funcall char-at (- (length chunk-body)
+								    1 ;; start of second delimiter
+								    1 ;; newline from second padline
+								    )
+							 chunk-body)))))
+      (if should-be-padded
+	  (if possibly-padded
+	      (if actually-padded
+		  (let* ((start-offset (+ 1 ;; end of first delimiter
+					  1 ;; newline from first padline
+					  1 ;; first character of body
+					  ))
+			 (end-offset (+ 1 ;; newline from second padline
+					1 ;; eol character after last character of body
+					))
+			 (start-index (- start-offset 1))
+			 (end-index (- (length chunk-body) end-offset))
+			 (end-inclusive-substring (lambda (str start end) (substring str start (+ 1 end))))
+			 (depadded-body (substring chunk-body start-index end-index))
+			 (distance-from-start (- desired-point chunk-start))
+			 (on-first-padline (= distance-from-start 1))
+			 (on-last-padline (= distance-from-start (- (length chunk-body) 1)))
+			 (adjusted-distance(cond (on-first-padline 0)
+						 (on-last-padline (length depadded-body))
+						 (t (- distance-from-start (- start-offset
+									      1 ;; compensate for the
+									        ;; fact that start-offset
+									        ;; includes the first
+									        ;; character
+									      )))))
+			 (distance-within-org-body (< adjusted-distance (- (length org-body) 1))))
+		    ;; If this chunk and the org source chunk match
+		    ;; send this chunk back and preserve the desired point
+		    (if (string= org-body depadded-body)
+			`(,depadded-body . ,adjusted-distance)
+		      ;; if the chunks match before the desired point
+		      ;; we can still preserve the point
+		      (if (and distance-within-org-body
+			       (string= (substring org-body 0 adjusted-distance)
+					(substring depadded-body 0 adjusted-distance)))
+			  `(,depadded-body . ,adjusted-distance)
+			;; otherwise we can't preserve the point
+			`(,depadded-body . 0))
+		      `(,depadded-body . 0)))
+		;; The user didn't respect the padlines
+		;; so the whole body goes back unchanged
+		`(,chunk-body . 0))
+	    `(,chunk-body . 0))
+	;; No adjustment needed
+	`(,chunk-body . ,desired-point)))))
+
 (provide 'ob-tangle)
 
 ;; Local variables:
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: Jumping from source block to Org block ...
  2013-09-14  3:04 Jumping from source block to Org block aditya siram
@ 2013-09-14 15:44 ` Eric Schulte
  2013-09-14 15:53   ` aditya siram
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Schulte @ 2013-09-14 15:44 UTC (permalink / raw)
  To: aditya siram; +Cc: emacs-orgmode

aditya siram <aditya.siram@gmail.com> writes:

> Attached is a patch that fixes a bug with jumping from source block back to
> the Org file. The problem is that the current detangling behavior does not
> take the :padlline flag into account. This stopped.
>
> Hopefully this is helpful to others ...
> -deech
>

Hi deech,

Please see the Org-mode contribution instructions at [1].  A patch of
this length would require that you fill out the FSF copyright assignment
paperwork before the patch could be applied.

As for the content of the patch, my only question is why do you add an
optional maintain-point argument to `org-babel-tangle-jump-to-org'?  Is
there ever a case when you would not want to maintain the point?

Of much less importance I have a couple of stylistic notes about the
code which are largely unrelated to its functionality and are included
to make future changes easier to read and because I'm a cranky old lisp
programmer.

- you should indent the code s.t. no lines are longer than 79 characters
- comments which float after code (e.g., ";; end of first delimiter")
  should only use 1 ; character
- the series of if statements (if should-be-padded... if
  possibly-padded... if actually-padded...) would be more legible if
  written as a single `cond' form.

Thanks for this change.  It appears to pass all tests, so after the
above have been addressed I'd be very happy to apply it.

Thanks for contributing, this is much appreciated!

If you have the time and inclination to include a test which fails
without this patch applied that would be icing on the cake.

Best,

Footnotes: 
[1]  http://orgmode.org/worg/org-contribute.html

-- 
Eric Schulte
https://cs.unm.edu/~eschulte
PGP: 0x614CA05D

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Jumping from source block to Org block ...
  2013-09-14 15:44 ` Eric Schulte
@ 2013-09-14 15:53   ` aditya siram
  2013-09-14 16:56     ` aditya siram
  2013-09-15 11:52     ` Eric Schulte
  0 siblings, 2 replies; 6+ messages in thread
From: aditya siram @ 2013-09-14 15:53 UTC (permalink / raw)
  To: Eric Schulte; +Cc: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 2235 bytes --]

Thanks for your feedback and your work on org-babel!

Oops, the maintain-point was a hold-over and isn't actually used in the
code. I'll remove it.

I will incorporate your suggestions.

However, regarding the cascading if statements, how would I use `cond` when
the predicates are `and`ed and when I need different behavior in the else
cases?


On Sat, Sep 14, 2013 at 10:44 AM, Eric Schulte <schulte.eric@gmail.com>wrote:

> aditya siram <aditya.siram@gmail.com> writes:
>
> > Attached is a patch that fixes a bug with jumping from source block back
> to
> > the Org file. The problem is that the current detangling behavior does
> not
> > take the :padlline flag into account. This stopped.
> >
> > Hopefully this is helpful to others ...
> > -deech
> >
>
> Hi deech,
>
> Please see the Org-mode contribution instructions at [1].  A patch of
> this length would require that you fill out the FSF copyright assignment
> paperwork before the patch could be applied.
>
> As for the content of the patch, my only question is why do you add an
> optional maintain-point argument to `org-babel-tangle-jump-to-org'?  Is
> there ever a case when you would not want to maintain the point?
>
> Of much less importance I have a couple of stylistic notes about the
> code which are largely unrelated to its functionality and are included
> to make future changes easier to read and because I'm a cranky old lisp
> programmer.
>
> - you should indent the code s.t. no lines are longer than 79 characters
> - comments which float after code (e.g., ";; end of first delimiter")
>   should only use 1 ; character
> - the series of if statements (if should-be-padded... if
>   possibly-padded... if actually-padded...) would be more legible if
>   written as a single `cond' form.
>
> Thanks for this change.  It appears to pass all tests, so after the
> above have been addressed I'd be very happy to apply it.
>
> Thanks for contributing, this is much appreciated!
>
> If you have the time and inclination to include a test which fails
> without this patch applied that would be icing on the cake.
>
> Best,
>
> Footnotes:
> [1]  http://orgmode.org/worg/org-contribute.html
>
> --
> Eric Schulte
> https://cs.unm.edu/~eschulte
> PGP: 0x614CA05D
>

[-- Attachment #2: Type: text/html, Size: 3101 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Jumping from source block to Org block ...
  2013-09-14 15:53   ` aditya siram
@ 2013-09-14 16:56     ` aditya siram
  2013-09-15 11:51       ` Eric Schulte
  2013-09-15 11:52     ` Eric Schulte
  1 sibling, 1 reply; 6+ messages in thread
From: aditya siram @ 2013-09-14 16:56 UTC (permalink / raw)
  To: Eric Schulte; +Cc: emacs-orgmode


[-- Attachment #1.1: Type: text/plain, Size: 2687 bytes --]

I've included new versions of both patches with most of the changes you
suggested. I guess you'll apply the longer one when you've been notified by
the FSF?

Is this a one-time deal that covers future patches or do I have to do this
with every patch that's over 15 lines long?

Thanks!
-deech


On Sat, Sep 14, 2013 at 10:53 AM, aditya siram <aditya.siram@gmail.com>wrote:

> Thanks for your feedback and your work on org-babel!
>
> Oops, the maintain-point was a hold-over and isn't actually used in the
> code. I'll remove it.
>
> I will incorporate your suggestions.
>
> However, regarding the cascading if statements, how would I use `cond`
> when the predicates are `and`ed and when I need different behavior in the
> else cases?
>
>
> On Sat, Sep 14, 2013 at 10:44 AM, Eric Schulte <schulte.eric@gmail.com>wrote:
>
>> aditya siram <aditya.siram@gmail.com> writes:
>>
>> > Attached is a patch that fixes a bug with jumping from source block
>> back to
>> > the Org file. The problem is that the current detangling behavior does
>> not
>> > take the :padlline flag into account. This stopped.
>> >
>> > Hopefully this is helpful to others ...
>> > -deech
>> >
>>
>> Hi deech,
>>
>> Please see the Org-mode contribution instructions at [1].  A patch of
>> this length would require that you fill out the FSF copyright assignment
>> paperwork before the patch could be applied.
>>
>> As for the content of the patch, my only question is why do you add an
>> optional maintain-point argument to `org-babel-tangle-jump-to-org'?  Is
>> there ever a case when you would not want to maintain the point?
>>
>> Of much less importance I have a couple of stylistic notes about the
>> code which are largely unrelated to its functionality and are included
>> to make future changes easier to read and because I'm a cranky old lisp
>> programmer.
>>
>> - you should indent the code s.t. no lines are longer than 79 characters
>> - comments which float after code (e.g., ";; end of first delimiter")
>>   should only use 1 ; character
>> - the series of if statements (if should-be-padded... if
>>   possibly-padded... if actually-padded...) would be more legible if
>>   written as a single `cond' form.
>>
>> Thanks for this change.  It appears to pass all tests, so after the
>> above have been addressed I'd be very happy to apply it.
>>
>> Thanks for contributing, this is much appreciated!
>>
>> If you have the time and inclination to include a test which fails
>> without this patch applied that would be icing on the cake.
>>
>> Best,
>>
>> Footnotes:
>> [1]  http://orgmode.org/worg/org-contribute.html
>>
>> --
>> Eric Schulte
>> https://cs.unm.edu/~eschulte
>> PGP: 0x614CA05D
>>
>
>

[-- Attachment #1.2: Type: text/html, Size: 3859 bytes --]

[-- Attachment #2: src_block_jump_fix.patch.txt --]
[-- Type: text/plain, Size: 5977 bytes --]

From 791a41a9cb97ae9a237b74c839a1fc2c0f4970db Mon Sep 17 00:00:00 2001
From: Aditya Siram <aditya siram at gmail dot com>
Date: Sat, 14 Sep 2013 11:47:17 -0500
Subject: [PATCH 2/2] Detangling and jumping back now correctly compensate for
 padded chunks

---
 lisp/ob-tangle.el | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 97 insertions(+), 5 deletions(-)

diff --git a/lisp/ob-tangle.el b/lisp/ob-tangle.el
index 8141943..42fa31c 100644
--- a/lisp/ob-tangle.el
+++ b/lisp/ob-tangle.el
@@ -506,8 +506,8 @@ which enable the original code blocks to be found."
   "Jump from a tangled code file to the related Org-mode file."
   (interactive)
   (let ((mid (point))
-	start body-start end done
-        target-buffer target-char link path block-name body)
+	start body-start end done depadded-body
+        target-buffer offset target-char link path block-name body)
     (save-window-excursion
       (save-excursion
 	(while (and (re-search-backward org-bracket-link-analytic-regexp nil t)
@@ -526,7 +526,7 @@ which enable the original code blocks to be found."
 			      (setq end (point-at-bol))))))))
 	(unless (and start (< start mid) (< mid end))
 	  (error "Not in tangled code"))
-        (setq body (org-babel-trim (buffer-substring start end))))
+        (setq body (buffer-substring start end)))
       (when (string-match "::" path)
         (setq path (substring path 0 (match-beginning 0))))
       (find-file path) (setq target-buffer (current-buffer))
@@ -537,12 +537,18 @@ which enable the original code blocks to be found."
         (org-babel-goto-named-src-block block-name))
       ;; position at the beginning of the code block body
       (goto-char (org-babel-where-is-src-block-head))
+      (let* ((pad-adjusted-values
+	      (org-babel-detangle-adjust-for-padlines start mid body))
+	     (depadded-body (car pad-adjusted-values))
+	     (depadded-point (cdr pad-adjusted-values)))
+	(progn
+	  (setq offset depadded-point)
+	  (setq body depadded-body)))
       (forward-line 1)
-      ;; Use org-edit-special to isolate the code.
       (org-edit-special)
       ;; Then move forward the correct number of characters in the
       ;; code buffer.
-      (forward-char (- mid body-start))
+      (forward-char offset)
       ;; And return to the Org-mode buffer with the point in the right
       ;; place.
       (org-edit-src-exit)
@@ -550,6 +556,92 @@ which enable the original code blocks to be found."
     (org-src-switch-to-buffer target-buffer t)
     (prog1 body (goto-char target-char))))
 
+(defun org-babel-detangle-adjust-for-padlines (chunk-start desired-point chunk-body)
+  "Check if :padline was enabled for this chunk and then
+adjust point and body accordingly. Also account for the user
+editing the tangled file and removing the padded lines.
+
+Returns a tuple (body . offset) where body is an adjusted source chunk
+with padded newlines possibly removed and offset is how far into
+that chunk point is. If point was on either of the padlines, point
+is set to the first or last character of the chunk depending on what's
+closer."
+  (save-excursion
+    (let* ((org-block (org-babel-get-src-block-info))
+	   (params (nth 2 org-block))
+	   (org-body (nth 1 org-block))
+	   (should-be-padded (not (string= "no" (cdr (assoc :padline params)))))
+	   (minimal-padded-chunk-length (+ 1 ; end of first delimiter
+					   1 ; newline from first padline
+					   1 ; newline from second padline
+					   1 ; start of second delimiter
+					   ))
+	   (possibly-padded (>= (length chunk-body) minimal-padded-chunk-length))
+	   (char-at (lambda (pos str) (substring str pos (+ pos 1))))
+	   (actually-padded  (and (not (=  (length chunk-body) 0))
+				  (string= "\n" (funcall char-at
+							 1
+							 chunk-body))
+				  (string= "\n" (funcall char-at
+							 (- (length chunk-body)
+							    1 ; start of second delimiter
+							    1 ; newline from second padline
+							    )
+							 chunk-body)))))
+      (if should-be-padded
+	  (if possibly-padded
+	      (if actually-padded
+		  (let* ((start-offset (+ 1 ; end of first delimiter
+					  1 ; newline from first padline
+					  1 ; first character of body
+					  ))
+			 (end-offset (+ 1 ; newline from second padline
+					1 ; eol character after last character of body
+					))
+			 (start-index (- start-offset 1))
+			 (end-index (- (length chunk-body) end-offset))
+			 (depadded-body (substring chunk-body
+						   start-index
+						   end-index))
+			 (distance-from-start (- desired-point chunk-start))
+			 (on-first-padline (= distance-from-start 1))
+			 (on-last-padline (= distance-from-start
+					     (- (length chunk-body) 1)))
+			 (adjusted-distance
+			  (cond (on-first-padline 0)
+				(on-last-padline (length depadded-body))
+				(t (- distance-from-start
+				      (- start-offset
+					 1 ; compensate for the fact that
+					   ; start-offset includes the first
+					   ; character
+					 )))))
+			 (distance-within-org-body (< adjusted-distance
+						      (- (length org-body) 1))))
+		    ;; If this chunk and the org source chunk match
+		    ;; send this chunk back and preserve the desired point
+		    (if (string= org-body depadded-body)
+			`(,depadded-body . ,adjusted-distance)
+		      ;; if the chunks match before the desired point
+		      ;; we can still preserve the point
+		      (if (and distance-within-org-body
+			       (string= (substring org-body
+						   0
+						   adjusted-distance)
+					(substring depadded-body
+						   0
+						   adjusted-distance)))
+			  `(,depadded-body . ,adjusted-distance)
+			;; otherwise we can't preserve the point
+			`(,depadded-body . 0))
+		      `(,depadded-body . 0)))
+		;; The user didn't respect the padlines
+		;; so the whole body goes back unchanged
+		`(,chunk-body . 0))
+	    `(,chunk-body . 0))
+	;; No adjustment needed
+	`(,chunk-body . ,desired-point)))))
+
 (provide 'ob-tangle)
 
 ;; Local variables:
-- 
1.8.1.2


[-- Attachment #3: src_regexp_fix.patch.txt --]
[-- Type: text/plain, Size: 987 bytes --]

From cce0cf9682c01c8ff4d36d0bf06df223b7c7cb6e Mon Sep 17 00:00:00 2001
From: Aditya Siram <aditya siram at gmail dot com>
Date: Fri, 13 Sep 2013 09:57:25 -0500
Subject: [PATCH 1/2] org-babel-src-block-regexp ignores empty body

 org-babel-src-block gobbled up everything until the ending delimiter
 of the next code block. Fixed this by making the body regexp non-greedy.

 lisp/ob-core.el | 2 +
 1 file changed, 1 insertion(+), 1 deletion(-)
---
 lisp/ob-core.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index d57806b..e8f16a0 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -188,7 +188,7 @@ This string must include a \"%s\" which will be replaced by the results."
    ;; (4) header arguments
    "\\([^\n]*\\)\n"
    ;; (5) body
-   "\\([^\000]*?\n\\)?[ \t]*#\\+end_src")
+   "\\([^\000]*?\n\\)??[ \t]*#\\+end_src")
   "Regexp used to identify code blocks.")
 
 (defvar org-babel-inline-src-block-regexp
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: Jumping from source block to Org block ...
  2013-09-14 16:56     ` aditya siram
@ 2013-09-15 11:51       ` Eric Schulte
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Schulte @ 2013-09-15 11:51 UTC (permalink / raw)
  To: aditya siram; +Cc: emacs-orgmode

aditya siram <aditya.siram@gmail.com> writes:

> I've included new versions of both patches with most of the changes you
> suggested. I guess you'll apply the longer one when you've been notified by
> the FSF?
>

Once you receive your confirmation from the FSF please let Bastien or
Carsten know, and they'll add you to the list of contributors at [1].
Then I'll apply this patch.

>
> Is this a one-time deal that covers future patches or do I have to do this
> with every patch that's over 15 lines long?
>

This is a one time deal that covers not only future patches to Org-mode,
but future patches to any part of Emacs.

Thanks again,

>
> Thanks!
> -deech
>
>
> On Sat, Sep 14, 2013 at 10:53 AM, aditya siram <aditya.siram@gmail.com>wrote:
>
>> Thanks for your feedback and your work on org-babel!
>>
>> Oops, the maintain-point was a hold-over and isn't actually used in the
>> code. I'll remove it.
>>
>> I will incorporate your suggestions.
>>
>> However, regarding the cascading if statements, how would I use `cond`
>> when the predicates are `and`ed and when I need different behavior in the
>> else cases?
>>
>>
>> On Sat, Sep 14, 2013 at 10:44 AM, Eric Schulte <schulte.eric@gmail.com>wrote:
>>
>>> aditya siram <aditya.siram@gmail.com> writes:
>>>
>>> > Attached is a patch that fixes a bug with jumping from source block
>>> back to
>>> > the Org file. The problem is that the current detangling behavior does
>>> not
>>> > take the :padlline flag into account. This stopped.
>>> >
>>> > Hopefully this is helpful to others ...
>>> > -deech
>>> >
>>>
>>> Hi deech,
>>>
>>> Please see the Org-mode contribution instructions at [1].  A patch of
>>> this length would require that you fill out the FSF copyright assignment
>>> paperwork before the patch could be applied.
>>>
>>> As for the content of the patch, my only question is why do you add an
>>> optional maintain-point argument to `org-babel-tangle-jump-to-org'?  Is
>>> there ever a case when you would not want to maintain the point?
>>>
>>> Of much less importance I have a couple of stylistic notes about the
>>> code which are largely unrelated to its functionality and are included
>>> to make future changes easier to read and because I'm a cranky old lisp
>>> programmer.
>>>
>>> - you should indent the code s.t. no lines are longer than 79 characters
>>> - comments which float after code (e.g., ";; end of first delimiter")
>>>   should only use 1 ; character
>>> - the series of if statements (if should-be-padded... if
>>>   possibly-padded... if actually-padded...) would be more legible if
>>>   written as a single `cond' form.
>>>
>>> Thanks for this change.  It appears to pass all tests, so after the
>>> above have been addressed I'd be very happy to apply it.
>>>
>>> Thanks for contributing, this is much appreciated!
>>>
>>> If you have the time and inclination to include a test which fails
>>> without this patch applied that would be icing on the cake.
>>>
>>> Best,
>>>
>>> Footnotes:
>>> [1]  http://orgmode.org/worg/org-contribute.html
>>>
>>> --
>>> Eric Schulte
>>> https://cs.unm.edu/~eschulte
>>> PGP: 0x614CA05D
>>>
>>
>>
>
>


Footnotes: 
[1]  http://orgmode.org/worg/org-contribute.html

-- 
Eric Schulte
https://cs.unm.edu/~eschulte
PGP: 0x614CA05D

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Jumping from source block to Org block ...
  2013-09-14 15:53   ` aditya siram
  2013-09-14 16:56     ` aditya siram
@ 2013-09-15 11:52     ` Eric Schulte
  1 sibling, 0 replies; 6+ messages in thread
From: Eric Schulte @ 2013-09-15 11:52 UTC (permalink / raw)
  To: aditya siram; +Cc: emacs-orgmode

aditya siram <aditya.siram@gmail.com> writes:

> Thanks for your feedback and your work on org-babel!
>

I'm happy you're finding it useful.

>
> Oops, the maintain-point was a hold-over and isn't actually used in the
> code. I'll remove it.
>
> I will incorporate your suggestions.
>
> However, regarding the cascading if statements, how would I use `cond` when
> the predicates are `and`ed and when I need different behavior in the else
> cases?
>

Maybe cascading if statements are the best thing here.  Perhaps changing
the order of the then/else branches could ensure the conditionals are
still visible from the branches, but this is an exceedingly minor point.

I guess any use of cond could easily get overly verbose.  Please
disregard this suggestion.

Cheers,

>
>
> On Sat, Sep 14, 2013 at 10:44 AM, Eric Schulte <schulte.eric@gmail.com>wrote:
>
>> aditya siram <aditya.siram@gmail.com> writes:
>>
>> > Attached is a patch that fixes a bug with jumping from source block back
>> to
>> > the Org file. The problem is that the current detangling behavior does
>> not
>> > take the :padlline flag into account. This stopped.
>> >
>> > Hopefully this is helpful to others ...
>> > -deech
>> >
>>
>> Hi deech,
>>
>> Please see the Org-mode contribution instructions at [1].  A patch of
>> this length would require that you fill out the FSF copyright assignment
>> paperwork before the patch could be applied.
>>
>> As for the content of the patch, my only question is why do you add an
>> optional maintain-point argument to `org-babel-tangle-jump-to-org'?  Is
>> there ever a case when you would not want to maintain the point?
>>
>> Of much less importance I have a couple of stylistic notes about the
>> code which are largely unrelated to its functionality and are included
>> to make future changes easier to read and because I'm a cranky old lisp
>> programmer.
>>
>> - you should indent the code s.t. no lines are longer than 79 characters
>> - comments which float after code (e.g., ";; end of first delimiter")
>>   should only use 1 ; character
>> - the series of if statements (if should-be-padded... if
>>   possibly-padded... if actually-padded...) would be more legible if
>>   written as a single `cond' form.
>>
>> Thanks for this change.  It appears to pass all tests, so after the
>> above have been addressed I'd be very happy to apply it.
>>
>> Thanks for contributing, this is much appreciated!
>>
>> If you have the time and inclination to include a test which fails
>> without this patch applied that would be icing on the cake.
>>
>> Best,
>>
>> Footnotes:
>> [1]  http://orgmode.org/worg/org-contribute.html
>>
>> --
>> Eric Schulte
>> https://cs.unm.edu/~eschulte
>> PGP: 0x614CA05D
>>

-- 
Eric Schulte
https://cs.unm.edu/~eschulte
PGP: 0x614CA05D

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-09-15 11:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-14  3:04 Jumping from source block to Org block aditya siram
2013-09-14 15:44 ` Eric Schulte
2013-09-14 15:53   ` aditya siram
2013-09-14 16:56     ` aditya siram
2013-09-15 11:51       ` Eric Schulte
2013-09-15 11:52     ` Eric Schulte

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