emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Saving the *Org Edit Src Example* buffer
@ 2009-06-02 15:02 Dan Davison
  2009-06-02 15:49 ` Dan Davison
  2009-06-02 17:04 ` Carsten Dominik
  0 siblings, 2 replies; 15+ messages in thread
From: Dan Davison @ 2009-06-02 15:02 UTC (permalink / raw)
  To: emacs org-mode mailing list

Following on from the recent improvements to the *Org Edit Src Example*
buffer, I have one more proposal: I have remapped C-x C-s so that it
saves the code in the org buffer, rather than offering to save the Edit
buffer itself (as it used to be with the indirect edit buffer). I find
this essential, although I recognise that remapping C-x C-s is a rather
radical thing to do to an emacs buffer.

I am using the simple-minded approach below; it seems to work fine (I
don't even notice a flicker -- should I be surprised at that?), but if
someone can suggest an improved implementation I'd be happy to learn how
it should be done (perhaps there are buffer variables other than point
and mark that I should restore? Is there a general mechanism I should
use for this?).

Dan

(defun org-edit-src-save ()
  "Update the parent org buffer with the edited source code, save
the parent org-buffer, and return to the source code edit
buffer."
  (interactive)
  (let ((p (point))
	(m (mark)))
    (org-edit-src-exit)
    (save-buffer)
    (org-edit-src-code)
    (set-mark m)
    (goto-char p)))

(define-key org-exit-edit-mode-map "\C-x\C-s" 'org-edit-src-save)

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

* Re: Saving the *Org Edit Src Example* buffer
  2009-06-02 15:02 Saving the *Org Edit Src Example* buffer Dan Davison
@ 2009-06-02 15:49 ` Dan Davison
  2009-06-02 17:12   ` Carsten Dominik
  2009-06-02 17:04 ` Carsten Dominik
  1 sibling, 1 reply; 15+ messages in thread
From: Dan Davison @ 2009-06-02 15:49 UTC (permalink / raw)
  To: emacs org-mode mailing list

Dan Davison <davison@stats.ox.ac.uk> writes:

> Following on from the recent improvements to the *Org Edit Src Example*
> buffer, I have one more proposal: I have remapped C-x C-s so that it
> saves the code in the org buffer, rather than offering to save the Edit
> buffer itself (as it used to be with the indirect edit buffer). I find
> this essential, although I recognise that remapping C-x C-s is a rather
> radical thing to do to an emacs buffer.
>
> I am using the simple-minded approach below; it seems to work fine (I

Hmm, well I had used that for at least a week before posting, but I have
now noticed a problem. It's a bit of a corner case. If you create extra
blank lines at the end of the source code edit buffer and leave point
down there, then do org-edit-src-exit, now back in the org buffer point
is outside the source code block. In that case, the code that I just
posted doesn't work. I suggest that org-edit-src-exit should in this
situation place point at the end of the source code block, for example
like this:


diff --git a/lisp/org.el b/lisp/org.el
index 659dfad..be459b5 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -6757,7 +6757,9 @@ the language, a switch telling of the content should be in a single line."
        code)
   (goto-char (point-min))
   (if (looking-at "[ \t\n]*\n") (replace-match ""))
-  (if (re-search-forward "\n[ \t\n]*\\'" nil t) (replace-match ""))
+  (when (re-search-forward "\n[ \t\n]*\\'" nil t)
+    (replace-match "")
+    (setq line (min line (org-current-line))))
   (when (org-bound-and-true-p org-edit-src-force-single-line)
     (goto-char (point-min))
     (while (re-search-forward "\n" nil t)


Dan




> don't even notice a flicker -- should I be surprised at that?), but if
> someone can suggest an improved implementation I'd be happy to learn how
> it should be done (perhaps there are buffer variables other than point
> and mark that I should restore? Is there a general mechanism I should
> use for this?).
>
> Dan
>
> (defun org-edit-src-save ()
>   "Update the parent org buffer with the edited source code, save
> the parent org-buffer, and return to the source code edit
> buffer."
>   (interactive)
>   (let ((p (point))
> 	(m (mark)))
>     (org-edit-src-exit)
>     (save-buffer)
>     (org-edit-src-code)
>     (set-mark m)
>     (goto-char p)))
>
> (define-key org-exit-edit-mode-map "\C-x\C-s" 'org-edit-src-save)
>
>
>
> _______________________________________________
> Emacs-orgmode mailing list
> Remember: use `Reply All' to send replies to the list.
> Emacs-orgmode@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-orgmode

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

* Re: Saving the *Org Edit Src Example* buffer
  2009-06-02 15:02 Saving the *Org Edit Src Example* buffer Dan Davison
  2009-06-02 15:49 ` Dan Davison
@ 2009-06-02 17:04 ` Carsten Dominik
  2009-08-11 16:44   ` PATCH: proposed improvements to org-src-mode Dan Davison
  1 sibling, 1 reply; 15+ messages in thread
From: Carsten Dominik @ 2009-06-02 17:04 UTC (permalink / raw)
  To: Dan Davison; +Cc: emacs org-mode mailing list

Hi Dan,

On Jun 2, 2009, at 5:02 PM, Dan Davison wrote:

> Following on from the recent improvements to the *Org Edit Src  
> Example*
> buffer, I have one more proposal: I have remapped C-x C-s so that it
> saves the code in the org buffer, rather than offering to save the  
> Edit
> buffer itself (as it used to be with the indirect edit buffer). I find
> this essential, although I recognise that remapping C-x C-s is a  
> rather
> radical thing to do to an emacs buffer.


But allowed:  From the Emacs lisp docs, under Major Mode Conventions:

      It is legitimate for a major mode to rebind a standard key
      sequence if it provides a command that does "the same job" in a
      way better suited to the text this mode is used for.

I'd say, your's is a perfect example for this rule.


> I am using the simple-minded approach below; it seems to work fine (I
> don't even notice a flicker -- should I be surprised at that?), but if
> someone can suggest an improved implementation I'd be happy to learn  
> how
> it should be done (perhaps there are buffer variables other than point
> and mark that I should restore? Is there a general mechanism I should
> use for this?).
>
> Dan
>
> (defun org-edit-src-save ()
>  "Update the parent org buffer with the edited source code, save
> the parent org-buffer, and return to the source code edit
> buffer."
>  (interactive)
>  (let ((p (point))
> 	(m (mark)))
>    (org-edit-src-exit)
>    (save-buffer)
>    (org-edit-src-code)
>    (set-mark m)
>    (goto-char p)))

This is already excellent.  I have changed it only slightly,
in order to get a better message when this command finishes, and
because `push-mark' should be used here instead of `set-mark'.

(defun org-edit-src-save ()
   "Save parent buffer with current state source-code buffer."
   (interactive)
   (let ((p (point)) (m (mark)) msg)
     (org-edit-src-exit)
     (save-buffer)
     (setq msg (current-message))
     (org-edit-src-code)
     (push-mark m 'nomessage)
     (goto-char p)
     (message (or msg ""))))

I have added your code, thanks.

- Carsten

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

* Re: Saving the *Org Edit Src Example* buffer
  2009-06-02 15:49 ` Dan Davison
@ 2009-06-02 17:12   ` Carsten Dominik
  2009-06-09 14:51     ` Dan Davison
  0 siblings, 1 reply; 15+ messages in thread
From: Carsten Dominik @ 2009-06-02 17:12 UTC (permalink / raw)
  To: Dan Davison; +Cc: emacs org-mode mailing list

Applied, thanks.

- Carsten

On Jun 2, 2009, at 5:49 PM, Dan Davison wrote:

> Dan Davison <davison@stats.ox.ac.uk> writes:
>
>> Following on from the recent improvements to the *Org Edit Src  
>> Example*
>> buffer, I have one more proposal: I have remapped C-x C-s so that it
>> saves the code in the org buffer, rather than offering to save the  
>> Edit
>> buffer itself (as it used to be with the indirect edit buffer). I  
>> find
>> this essential, although I recognise that remapping C-x C-s is a  
>> rather
>> radical thing to do to an emacs buffer.
>>
>> I am using the simple-minded approach below; it seems to work fine (I
>
> Hmm, well I had used that for at least a week before posting, but I  
> have
> now noticed a problem. It's a bit of a corner case. If you create  
> extra
> blank lines at the end of the source code edit buffer and leave point
> down there, then do org-edit-src-exit, now back in the org buffer  
> point
> is outside the source code block. In that case, the code that I just
> posted doesn't work. I suggest that org-edit-src-exit should in this
> situation place point at the end of the source code block, for example
> like this:
>
>
> diff --git a/lisp/org.el b/lisp/org.el
> index 659dfad..be459b5 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -6757,7 +6757,9 @@ the language, a switch telling of the content  
> should be in a single line."
>        code)
>   (goto-char (point-min))
>   (if (looking-at "[ \t\n]*\n") (replace-match ""))
> -  (if (re-search-forward "\n[ \t\n]*\\'" nil t) (replace-match ""))
> +  (when (re-search-forward "\n[ \t\n]*\\'" nil t)
> +    (replace-match "")
> +    (setq line (min line (org-current-line))))
>   (when (org-bound-and-true-p org-edit-src-force-single-line)
>     (goto-char (point-min))
>     (while (re-search-forward "\n" nil t)
>
>
> Dan
>
>
>
>
>> don't even notice a flicker -- should I be surprised at that?), but  
>> if
>> someone can suggest an improved implementation I'd be happy to  
>> learn how
>> it should be done (perhaps there are buffer variables other than  
>> point
>> and mark that I should restore? Is there a general mechanism I should
>> use for this?).
>>
>> Dan
>>
>> (defun org-edit-src-save ()
>>  "Update the parent org buffer with the edited source code, save
>> the parent org-buffer, and return to the source code edit
>> buffer."
>>  (interactive)
>>  (let ((p (point))
>> 	(m (mark)))
>>    (org-edit-src-exit)
>>    (save-buffer)
>>    (org-edit-src-code)
>>    (set-mark m)
>>    (goto-char p)))
>>
>> (define-key org-exit-edit-mode-map "\C-x\C-s" 'org-edit-src-save)
>>
>>
>>
>> _______________________________________________
>> Emacs-orgmode mailing list
>> Remember: use `Reply All' to send replies to the list.
>> Emacs-orgmode@gnu.org
>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode
>
>
> _______________________________________________
> Emacs-orgmode mailing list
> Remember: use `Reply All' to send replies to the list.
> Emacs-orgmode@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-orgmode

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

* Re: Saving the *Org Edit Src Example* buffer
  2009-06-02 17:12   ` Carsten Dominik
@ 2009-06-09 14:51     ` Dan Davison
  2009-06-09 18:20       ` Carsten Dominik
  0 siblings, 1 reply; 15+ messages in thread
From: Dan Davison @ 2009-06-09 14:51 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: emacs org-mode mailing list

Carsten Dominik <carsten.dominik@gmail.com> writes:

> Applied, thanks.

Sorry to go on about this, but I don't think the patch that I sent has
been applied and I believe that the problem it addresses still exists:
go into a source code block, hit C-c ', go to end of edit buffer, press
return a few times, try C-x C-s and you'll either be back in the org
buffer (or if you're really unlucky in an edit buffer for a different
code block!). This occurs because when we drop out of the edit buffer,
point ends up outside the code block in the parent org buffer. I've now
realised that a similar problem occurs if you add blank lines at the
top, and so my original patch was insufficient.

My current patch follows. It's not perfect: if there are leading blank
lines then after C-x C-s point will end up in a different place, but at
least it's in the correct buffer :) It seems to me that making point
stay still would require org-edit-src-exit to inform org-edit-src-save
about exactly how many whitespace characters it has removed, and I
haven't tried to do that.

Dan

diff --git a/lisp/org-src.el b/lisp/org-src.el
index 401c628..f4559f2 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -403,18 +403,19 @@ the language, a switch telling of the content should be in a single line."
   (interactive)
   (unless (string-match "\\`*Org Edit " (buffer-name (current-buffer)))
     (error "This is not an sub-editing buffer, something is wrong..."))
-  (let ((line (if (org-bound-and-true-p org-edit-src-force-single-line)
-                 1
-               (org-current-line)))
-       (beg org-edit-src-beg-marker)
+  (let ((beg org-edit-src-beg-marker)
        (end org-edit-src-end-marker)
        (ovl org-edit-src-overlay)
        (buffer (current-buffer))
        (nindent org-edit-src-nindent)
-       code)
-  (goto-char (point-min))
-  (if (looking-at "[ \t\n]*\n") (replace-match ""))
-  (if (re-search-forward "\n[ \t\n]*\\'" nil t) (replace-match ""))
+       line code)
+  (save-excursion
+    (goto-char (point-min))
+    (if (looking-at "[ \t\n]*\n") (replace-match ""))
+    (if (re-search-forward "\n[ \t\n]*\\'" nil t) (replace-match "")))
+  (setq line (if (org-bound-and-true-p org-edit-src-force-single-line)
+                1
+              (org-current-line)))
   (when (org-bound-and-true-p org-edit-src-force-single-line)
     (goto-char (point-min))
     (while (re-search-forward "\n" nil t)

Dan

>
> - Carsten
>
> On Jun 2, 2009, at 5:49 PM, Dan Davison wrote:
>
>> Dan Davison <davison@stats.ox.ac.uk> writes:
>>
>>> Following on from the recent improvements to the *Org Edit Src
>>> Example*
>>> buffer, I have one more proposal: I have remapped C-x C-s so that it
>>> saves the code in the org buffer, rather than offering to save the
>>> Edit
>>> buffer itself (as it used to be with the indirect edit buffer). I
>>> find
>>> this essential, although I recognise that remapping C-x C-s is a
>>> rather
>>> radical thing to do to an emacs buffer.
>>>
>>> I am using the simple-minded approach below; it seems to work fine (I
>>
>> Hmm, well I had used that for at least a week before posting, but I
>> have
>> now noticed a problem. It's a bit of a corner case. If you create
>> extra
>> blank lines at the end of the source code edit buffer and leave point
>> down there, then do org-edit-src-exit, now back in the org buffer
>> point
>> is outside the source code block. In that case, the code that I just
>> posted doesn't work. I suggest that org-edit-src-exit should in this
>> situation place point at the end of the source code block, for example
>> like this:
>>
>>
>> diff --git a/lisp/org.el b/lisp/org.el
>> index 659dfad..be459b5 100644
>> --- a/lisp/org.el
>> +++ b/lisp/org.el
>> @@ -6757,7 +6757,9 @@ the language, a switch telling of the content
>> should be in a single line."
>>        code)
>>   (goto-char (point-min))
>>   (if (looking-at "[ \t\n]*\n") (replace-match ""))
>> -  (if (re-search-forward "\n[ \t\n]*\\'" nil t) (replace-match ""))
>> +  (when (re-search-forward "\n[ \t\n]*\\'" nil t)
>> +    (replace-match "")
>> +    (setq line (min line (org-current-line))))
>>   (when (org-bound-and-true-p org-edit-src-force-single-line)
>>     (goto-char (point-min))
>>     (while (re-search-forward "\n" nil t)
>>
>>
>> Dan
>>
>>
>>
>>
>>> don't even notice a flicker -- should I be surprised at that?), but
>>> if
>>> someone can suggest an improved implementation I'd be happy to
>>> learn how
>>> it should be done (perhaps there are buffer variables other than
>>> point
>>> and mark that I should restore? Is there a general mechanism I should
>>> use for this?).
>>>
>>> Dan
>>>
>>> (defun org-edit-src-save ()
>>>  "Update the parent org buffer with the edited source code, save
>>> the parent org-buffer, and return to the source code edit
>>> buffer."
>>>  (interactive)
>>>  (let ((p (point))
>>> 	(m (mark)))
>>>    (org-edit-src-exit)
>>>    (save-buffer)
>>>    (org-edit-src-code)
>>>    (set-mark m)
>>>    (goto-char p)))
>>>
>>> (define-key org-exit-edit-mode-map "\C-x\C-s" 'org-edit-src-save)
>>>
>>>
>>>
>>> _______________________________________________
>>> Emacs-orgmode mailing list
>>> Remember: use `Reply All' to send replies to the list.
>>> Emacs-orgmode@gnu.org
>>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode
>>
>>
>> _______________________________________________
>> Emacs-orgmode mailing list
>> Remember: use `Reply All' to send replies to the list.
>> Emacs-orgmode@gnu.org
>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode
>
>
>
> _______________________________________________
> Emacs-orgmode mailing list
> Remember: use `Reply All' to send replies to the list.
> Emacs-orgmode@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-orgmode

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

* Re: Saving the *Org Edit Src Example* buffer
  2009-06-09 14:51     ` Dan Davison
@ 2009-06-09 18:20       ` Carsten Dominik
  0 siblings, 0 replies; 15+ messages in thread
From: Carsten Dominik @ 2009-06-09 18:20 UTC (permalink / raw)
  To: Dan Davison; +Cc: emacs org-mode mailing list


On Jun 9, 2009, at 4:51 PM, Dan Davison wrote:

> Carsten Dominik <carsten.dominik@gmail.com> writes:
>
>> Applied, thanks.
>
> Sorry to go on about this, but I don't think the patch that I sent has
> been applied and I believe that the problem it addresses still exists:
> go into a source code block, hit C-c ', go to end of edit buffer,  
> press
> return a few times, try C-x C-s and you'll either be back in the org
> buffer (or if you're really unlucky in an edit buffer for a different
> code block!). This occurs because when we drop out of the edit buffer,
> point ends up outside the code block in the parent org buffer. I've  
> now
> realised that a similar problem occurs if you add blank lines at the
> top, and so my original patch was insufficient.

You are right.

>
> My current patch follows. It's not perfect: if there are leading blank
> lines then after C-x C-s point will end up in a different place, but  
> at
> least it's in the correct buffer :) It seems to me that making point
> stay still would require org-edit-src-exit to inform org-edit-src-save
> about exactly how many whitespace characters it has removed, and I
> haven't tried to do that.

I think the better way to do this is to first remove the
empty lines at the beginning and end of the buffer, and then to
remember the line.  This is what I have implemented now.

- Carsten

>
> Dan
>
> diff --git a/lisp/org-src.el b/lisp/org-src.el
> index 401c628..f4559f2 100644
> --- a/lisp/org-src.el
> +++ b/lisp/org-src.el
> @@ -403,18 +403,19 @@ the language, a switch telling of the content  
> should be in a single line."
>   (interactive)
>   (unless (string-match "\\`*Org Edit " (buffer-name (current- 
> buffer)))
>     (error "This is not an sub-editing buffer, something is  
> wrong..."))
> -  (let ((line (if (org-bound-and-true-p org-edit-src-force-single- 
> line)
> -                 1
> -               (org-current-line)))
> -       (beg org-edit-src-beg-marker)
> +  (let ((beg org-edit-src-beg-marker)
>        (end org-edit-src-end-marker)
>        (ovl org-edit-src-overlay)
>        (buffer (current-buffer))
>        (nindent org-edit-src-nindent)
> -       code)
> -  (goto-char (point-min))
> -  (if (looking-at "[ \t\n]*\n") (replace-match ""))
> -  (if (re-search-forward "\n[ \t\n]*\\'" nil t) (replace-match ""))
> +       line code)
> +  (save-excursion
> +    (goto-char (point-min))
> +    (if (looking-at "[ \t\n]*\n") (replace-match ""))
> +    (if (re-search-forward "\n[ \t\n]*\\'" nil t) (replace-match  
> "")))
> +  (setq line (if (org-bound-and-true-p org-edit-src-force-single- 
> line)
> +                1
> +              (org-current-line)))
>   (when (org-bound-and-true-p org-edit-src-force-single-line)
>     (goto-char (point-min))
>     (while (re-search-forward "\n" nil t)
>
> Dan
>
>>
>> - Carsten
>>
>> On Jun 2, 2009, at 5:49 PM, Dan Davison wrote:
>>
>>> Dan Davison <davison@stats.ox.ac.uk> writes:
>>>
>>>> Following on from the recent improvements to the *Org Edit Src
>>>> Example*
>>>> buffer, I have one more proposal: I have remapped C-x C-s so that  
>>>> it
>>>> saves the code in the org buffer, rather than offering to save the
>>>> Edit
>>>> buffer itself (as it used to be with the indirect edit buffer). I
>>>> find
>>>> this essential, although I recognise that remapping C-x C-s is a
>>>> rather
>>>> radical thing to do to an emacs buffer.
>>>>
>>>> I am using the simple-minded approach below; it seems to work  
>>>> fine (I
>>>
>>> Hmm, well I had used that for at least a week before posting, but I
>>> have
>>> now noticed a problem. It's a bit of a corner case. If you create
>>> extra
>>> blank lines at the end of the source code edit buffer and leave  
>>> point
>>> down there, then do org-edit-src-exit, now back in the org buffer
>>> point
>>> is outside the source code block. In that case, the code that I just
>>> posted doesn't work. I suggest that org-edit-src-exit should in this
>>> situation place point at the end of the source code block, for  
>>> example
>>> like this:
>>>
>>>
>>> diff --git a/lisp/org.el b/lisp/org.el
>>> index 659dfad..be459b5 100644
>>> --- a/lisp/org.el
>>> +++ b/lisp/org.el
>>> @@ -6757,7 +6757,9 @@ the language, a switch telling of the content
>>> should be in a single line."
>>>       code)
>>>  (goto-char (point-min))
>>>  (if (looking-at "[ \t\n]*\n") (replace-match ""))
>>> -  (if (re-search-forward "\n[ \t\n]*\\'" nil t) (replace-match ""))
>>> +  (when (re-search-forward "\n[ \t\n]*\\'" nil t)
>>> +    (replace-match "")
>>> +    (setq line (min line (org-current-line))))
>>>  (when (org-bound-and-true-p org-edit-src-force-single-line)
>>>    (goto-char (point-min))
>>>    (while (re-search-forward "\n" nil t)
>>>
>>>
>>> Dan
>>>
>>>
>>>
>>>
>>>> don't even notice a flicker -- should I be surprised at that?), but
>>>> if
>>>> someone can suggest an improved implementation I'd be happy to
>>>> learn how
>>>> it should be done (perhaps there are buffer variables other than
>>>> point
>>>> and mark that I should restore? Is there a general mechanism I  
>>>> should
>>>> use for this?).
>>>>
>>>> Dan
>>>>
>>>> (defun org-edit-src-save ()
>>>> "Update the parent org buffer with the edited source code, save
>>>> the parent org-buffer, and return to the source code edit
>>>> buffer."
>>>> (interactive)
>>>> (let ((p (point))
>>>> 	(m (mark)))
>>>>   (org-edit-src-exit)
>>>>   (save-buffer)
>>>>   (org-edit-src-code)
>>>>   (set-mark m)
>>>>   (goto-char p)))
>>>>
>>>> (define-key org-exit-edit-mode-map "\C-x\C-s" 'org-edit-src-save)
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Emacs-orgmode mailing list
>>>> Remember: use `Reply All' to send replies to the list.
>>>> Emacs-orgmode@gnu.org
>>>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode
>>>
>>>
>>> _______________________________________________
>>> Emacs-orgmode mailing list
>>> Remember: use `Reply All' to send replies to the list.
>>> Emacs-orgmode@gnu.org
>>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode
>>
>>
>>
>> _______________________________________________
>> Emacs-orgmode mailing list
>> Remember: use `Reply All' to send replies to the list.
>> Emacs-orgmode@gnu.org
>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode

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

* PATCH: proposed improvements to org-src-mode
  2009-06-02 17:04 ` Carsten Dominik
@ 2009-08-11 16:44   ` Dan Davison
  2009-08-12 12:22     ` Carsten Dominik
  0 siblings, 1 reply; 15+ messages in thread
From: Dan Davison @ 2009-08-11 16:44 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: emacs org-mode mailing list

I'm attaching a patch which attempts to make some improvements to
org-src-mode. A quick recap: currently, C-c ' on a source code block
displays the code in a language major mode buffer with minor mode
org-src-mode, which features the following two useful key-bindings:

| C-x s | org-edit-src-save | save the code in the source code block in the parent org file |
| C-c ' | org-edit-src-exit | return to the parent org file with new code                   |

Furthermore, while the edit buffer is alive, the originating code block
is subject to a special overlay which links to the edit buffer when you
click on it. This is all excellent, and I use it every day, but I think
there's still a couple of improvements that we should make.

Specifically, I'm proposing that the following are bugs:

* Proposed bug I
      C-x k kills the edit buffer without questions; the overlay
      remains, but now links to a deleted buffer.
* Proposed bug II
      C-x C-c kills a modified edit buffer silently, without offering to
      save your work. I have lost work like that a number of times
      recently.
* Proposed bug III
      C-x s does not offer to save a modified edit buffer

The attached patch does the following.
- C-x s offers to save edit buffers
- C-x C-c offers to save edit buffers
- C-x k warns that you're killing an edit buffer
- If you do kill an edit buffer, the overlay in the parent buffer is removed
- Edit buffers are named *Org Src <orgbuf>[<lang>]*, where
  <orgbuf> is the name of the org-mode buffer containing this
  source code block, and lang is the language major mode.
- An internal detail is that org-edit-src-save is added to the
  write-contents-functions list, which means that it is no longer
  necessary to explicitly remap C-x C-s to org-edit-src-save

* Notes
  This patch gives the desired behaviour, at the cost of being forced to
  assign a buffer-file-name to the edit buffer. The consequence is that
  the edit buffer is considered to always be modified, since a file of
  that name is never actually written to (doesn't even exist). I didn't
  manage to come up with a way to trick emacs into holding the
  appropriate beliefs about whether the buffer had been modified. But in
  any case, I think there's an argument that these modifications
  warnings are a good thing, because one should not leave active edit
  buffers around: you should always have exited with C-c ' first.

Just in case it is helpful, I am including the notes I made in the
course of making these changes at the very bottom of the email.

Dan

p.s. In these two lines:
-  (unless (string-match "\\`*Org Edit " (buffer-name (current-buffer)))
-    (error "This is not an sub-editing buffer, something is wrong..."))
+  (unless org-edit-src-from-org-mode
+    (error "This is not a sub-editing buffer, something is wrong..."))

I assumed that org-edit-src-from-org-mode was an appropriate test. But
that may be incorrect as I am not certain what the intention was for
that variable.

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/org-src.el b/lisp/org-src.el
index 2a6c087..a5816d2 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -113,7 +113,6 @@ but which mess up the display of a snippet in Org exported files.")
 
 (defvar org-src-mode-map (make-sparse-keymap))
 (define-key org-src-mode-map "\C-c'" 'org-edit-src-exit)
-(define-key org-src-mode-map "\C-x\C-s" 'org-edit-src-save)
 (defvar org-edit-src-force-single-line nil)
 (defvar org-edit-src-from-org-mode nil)
 (defvar org-edit-src-picture nil)
@@ -168,7 +167,8 @@ the edited version."
 	    (if (boundp 'org-edit-src-overlay)
 		(org-delete-overlay org-edit-src-overlay)))
 	  (kill-buffer buffer))
-	(setq buffer (generate-new-buffer "*Org Edit Src Example*"))
+	(setq buffer (generate-new-buffer
+		      (concat "*Org Src " (file-name-nondirectory buffer-file-name) "[" lang "]*")))
 	(setq ovl (org-make-overlay beg end))
 	(org-overlay-put ovl 'face 'secondary-selection)
 	(org-overlay-put ovl 'edit-buffer buffer)
@@ -186,8 +186,7 @@ the edited version."
 				'(display nil invisible nil intangible nil))
 	(org-do-remove-indentation)
 	(let ((org-inhibit-startup t))
-	  (funcall lang-f)
-	  (org-src-mode))
+	  (funcall lang-f))
 	(set (make-local-variable 'org-edit-src-force-single-line) single)
 	(set (make-local-variable 'org-edit-src-from-org-mode) org-mode-p)
 	(when lfmt
@@ -201,6 +200,7 @@ the edited version."
 	(org-set-local 'org-edit-src-end-marker end)
 	(org-set-local 'org-edit-src-overlay ovl)
 	(org-set-local 'org-edit-src-nindent nindent)
+	(org-src-mode)
 	(and org-edit-src-persistent-message
 	     (org-set-local 'header-line-format msg)))
       (message "%s" msg)
@@ -400,12 +400,13 @@ the language, a switch telling of the content should be in a single line."
 (defun org-edit-src-exit ()
   "Exit special edit and protect problematic lines."
   (interactive)
-  (unless (string-match "\\`*Org Edit " (buffer-name (current-buffer)))
-    (error "This is not an sub-editing buffer, something is wrong..."))
+  (unless org-edit-src-from-org-mode
+    (error "This is not a sub-editing buffer, something is wrong..."))
   (let ((beg org-edit-src-beg-marker)
 	(end org-edit-src-end-marker)
 	(ovl org-edit-src-overlay)
 	(buffer (current-buffer))
+	(buffer-file-name nil)
 	(nindent org-edit-src-nindent)
 	code line)
     (untabify (point-min) (point-max))
@@ -444,7 +445,6 @@ the language, a switch telling of the content should be in a single line."
     (switch-to-buffer (marker-buffer beg))
     (kill-buffer buffer)
     (goto-char beg)
-    (org-delete-overlay ovl)
     (delete-region beg end)
     (insert code)
     (goto-char beg)
@@ -464,6 +464,18 @@ the language, a switch telling of the content should be in a single line."
     (goto-char (min p (point-max)))
     (message (or msg ""))))
 
+(defun org-src-mode-configure-buffer ()
+  (setq buffer-offer-save t)
+  (setq buffer-file-name
+	(concat (buffer-file-name (marker-buffer org-edit-src-beg-marker))
+		"[" (buffer-name) "]"))
+  (set (if (featurep 'xemacs) 'write-contents-hooks 'write-contents-functions)
+       '(org-edit-src-save))
+  (org-add-hook 'kill-buffer-hook
+		'(lambda () (org-delete-overlay org-edit-src-overlay)) nil 'local))
+
+(org-add-hook 'org-src-mode-hook 'org-src-mode-configure-buffer)
+
 (provide 'org-src)
 
 ;; arch-tag: 6a1fc84f-dec7-47be-a416-64be56bea5d8
--8<---------------cut here---------------end--------------->8---

* Notes on patch
*** write-contents-functions
    A good start seems to be to use org-src-mode-hook to add
    org-edit-src-save to the write-contents-functions list. This
    means that when it comes to saving, org-edit-src-save will be
    called and no subsequent attempt will be made to save the buffer
    in the normal way. (This should obviate the remapping of C-x C-s
    to org-edit-src-save in org-src.el)
*** buffer-offer-save
    We also want to set this to t.

*** Where does this get us?

    - C-x s still does *not* offer to save the edit buffer. That's
      because buffer-file-name is nil.
    
    - C-x C-c does ask us whether we want to save the
      edit buffer. However, since buffer-file-name is nil it asks us
      for a file name. The check in org-edit-src-exit throws an error
      unless the buffer is named '* Org Edit '...

    - C-x k kills the buffer silently, leaving a broken overlay
      link. If buffer-file-name were set, it would have warned that
      the buffer was modified.

*** buffer-file-name
    So, that all suggests that we need to set buffer-file-name, even
    though we don't really want to associate this buffer with a file
    in the normal way. As for the file name, my current suggestion
    is parent-org-filename[edit-buffer-name].
    
    [I had to move the (org-src-mode) call to the end of
    org-edit-src-code to make sure that the required variables were
    defined when the hook was called.]
    
*** And so where are we now?
    - C-x s *does* offer to save the edit buffer, but in saving
      produces a warning that the edit buffer is modified.
    - C-x k now gives a warning that the edit buffer is modified
      (even if it's not).
    - C-x C-c is working as desired, except that again we get
      warnings that the edit buffer is modified, once when we save,
      and again just before exiting emacs.
    - And C-c ' now issues a warning that the edit buffer is
      modified when we leave it, which we don't want.
*** So, we need to get rid of the buffer modification warnings.
    I've made buffer-file-name nil inside the let binding in
    org-edit-src-exit.
*** And?
    - C-x s behaves as desired, except that as was already the case,
      the edit buffer is always considered modified, and so repeated
      invocations keep saving it.
    - As was already the case, C-x k always gives a warning that the
      edit buffer has been modified.
    - C-x C-c is as desired (offers to save the edit buffer) except
      that it warns of the modified buffer just before exiting.
    - C-c ' is as it should be (silent)


Carsten Dominik <carsten.dominik@gmail.com> writes:

> Hi Dan,
>
> On Jun 2, 2009, at 5:02 PM, Dan Davison wrote:
>
>> Following on from the recent improvements to the *Org Edit Src
>> Example*
>> buffer, I have one more proposal: I have remapped C-x C-s so that it
>> saves the code in the org buffer, rather than offering to save the
>> Edit
>> buffer itself (as it used to be with the indirect edit buffer). I find
>> this essential, although I recognise that remapping C-x C-s is a
>> rather
>> radical thing to do to an emacs buffer.
>
>
> But allowed:  From the Emacs lisp docs, under Major Mode Conventions:
>
>      It is legitimate for a major mode to rebind a standard key
>      sequence if it provides a command that does "the same job" in a
>      way better suited to the text this mode is used for.
>
> I'd say, your's is a perfect example for this rule.
>
>
>> I am using the simple-minded approach below; it seems to work fine (I
>> don't even notice a flicker -- should I be surprised at that?), but if
>> someone can suggest an improved implementation I'd be happy to learn
>> how
>> it should be done (perhaps there are buffer variables other than point
>> and mark that I should restore? Is there a general mechanism I should
>> use for this?).
>>
>> Dan
>>
>> (defun org-edit-src-save ()
>>  "Update the parent org buffer with the edited source code, save
>> the parent org-buffer, and return to the source code edit
>> buffer."
>>  (interactive)
>>  (let ((p (point))
>> 	(m (mark)))
>>    (org-edit-src-exit)
>>    (save-buffer)
>>    (org-edit-src-code)
>>    (set-mark m)
>>    (goto-char p)))
>
> This is already excellent.  I have changed it only slightly,
> in order to get a better message when this command finishes, and
> because `push-mark' should be used here instead of `set-mark'.
>
> (defun org-edit-src-save ()
>   "Save parent buffer with current state source-code buffer."
>   (interactive)
>   (let ((p (point)) (m (mark)) msg)
>     (org-edit-src-exit)
>     (save-buffer)
>     (setq msg (current-message))
>     (org-edit-src-code)
>     (push-mark m 'nomessage)
>     (goto-char p)
>     (message (or msg ""))))
>
> I have added your code, thanks.
>
> - Carsten

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

* Re: PATCH: proposed improvements to org-src-mode
  2009-08-11 16:44   ` PATCH: proposed improvements to org-src-mode Dan Davison
@ 2009-08-12 12:22     ` Carsten Dominik
  2009-08-12 14:58       ` Dan Davison
  0 siblings, 1 reply; 15+ messages in thread
From: Carsten Dominik @ 2009-08-12 12:22 UTC (permalink / raw)
  To: Dan Davison; +Cc: emacs org-mode mailing list

Hi Dan,

thank you for studying and describing these issues, and for proposing  
a patch.

I am not sure that the implementation you offer is the cleanest
possibile, I definitely do not want to attach a file to this
temporary editing buffer.  It is probably better to install
a kill-buffer-hook to force the query, for example, or even
to advise the save-buffers-kill-terminal function to
handle the special case.

First of all, in reading your mail I have a few problems
understanding exactly what you mean, because I have the feeling
that you do not clearly distinguish between `C-x s' and `C-x C-s'.   
Because you write that `C-x s' is bound to `org-edit-src-save'
which is is not.

Could you please review your post to make sure that you
are using the correct keys?  The I will comment further.

Thanks.

- Carsten


On Aug 11, 2009, at 6:44 PM, Dan Davison wrote:

> I'm attaching a patch which attempts to make some improvements to
> org-src-mode. A quick recap: currently, C-c ' on a source code block
> displays the code in a language major mode buffer with minor mode
> org-src-mode, which features the following two useful key-bindings:
>
> | C-x s | org-edit-src-save | save the code in the source code block  
> in the parent org file |
> | C-c ' | org-edit-src-exit | return to the parent org file with new  
> code                   |
>
> Furthermore, while the edit buffer is alive, the originating code  
> block
> is subject to a special overlay which links to the edit buffer when  
> you
> click on it. This is all excellent, and I use it every day, but I  
> think
> there's still a couple of improvements that we should make.
>
> Specifically, I'm proposing that the following are bugs:
>
> * Proposed bug I
>      C-x k kills the edit buffer without questions; the overlay
>      remains, but now links to a deleted buffer.
> * Proposed bug II
>      C-x C-c kills a modified edit buffer silently, without offering  
> to
>      save your work. I have lost work like that a number of times
>      recently.
> * Proposed bug III
>      C-x s does not offer to save a modified edit buffer
>
> The attached patch does the following.
> - C-x s offers to save edit buffers
> - C-x C-c offers to save edit buffers
> - C-x k warns that you're killing an edit buffer
> - If you do kill an edit buffer, the overlay in the parent buffer is  
> removed
> - Edit buffers are named *Org Src <orgbuf>[<lang>]*, where
>  <orgbuf> is the name of the org-mode buffer containing this
>  source code block, and lang is the language major mode.
> - An internal detail is that org-edit-src-save is added to the
>  write-contents-functions list, which means that it is no longer
>  necessary to explicitly remap C-x C-s to org-edit-src-save
>
> * Notes
>  This patch gives the desired behaviour, at the cost of being forced  
> to
>  assign a buffer-file-name to the edit buffer. The consequence is that
>  the edit buffer is considered to always be modified, since a file of
>  that name is never actually written to (doesn't even exist). I didn't
>  manage to come up with a way to trick emacs into holding the
>  appropriate beliefs about whether the buffer had been modified. But  
> in
>  any case, I think there's an argument that these modifications
>  warnings are a good thing, because one should not leave active edit
>  buffers around: you should always have exited with C-c ' first.
>
> Just in case it is helpful, I am including the notes I made in the
> course of making these changes at the very bottom of the email.
>
> Dan
>
> p.s. In these two lines:
> -  (unless (string-match "\\`*Org Edit " (buffer-name (current- 
> buffer)))
> -    (error "This is not an sub-editing buffer, something is  
> wrong..."))
> +  (unless org-edit-src-from-org-mode
> +    (error "This is not a sub-editing buffer, something is  
> wrong..."))
>
> I assumed that org-edit-src-from-org-mode was an appropriate test. But
> that may be incorrect as I am not certain what the intention was for
> that variable.
>
> --8<---------------cut here---------------start------------->8---
> diff --git a/lisp/org-src.el b/lisp/org-src.el
> index 2a6c087..a5816d2 100644
> --- a/lisp/org-src.el
> +++ b/lisp/org-src.el
> @@ -113,7 +113,6 @@ but which mess up the display of a snippet in  
> Org exported files.")
>
> (defvar org-src-mode-map (make-sparse-keymap))
> (define-key org-src-mode-map "\C-c'" 'org-edit-src-exit)
> -(define-key org-src-mode-map "\C-x\C-s" 'org-edit-src-save)
> (defvar org-edit-src-force-single-line nil)
> (defvar org-edit-src-from-org-mode nil)
> (defvar org-edit-src-picture nil)
> @@ -168,7 +167,8 @@ the edited version."
> 	    (if (boundp 'org-edit-src-overlay)
> 		(org-delete-overlay org-edit-src-overlay)))
> 	  (kill-buffer buffer))
> -	(setq buffer (generate-new-buffer "*Org Edit Src Example*"))
> +	(setq buffer (generate-new-buffer
> +		      (concat "*Org Src " (file-name-nondirectory buffer-file- 
> name) "[" lang "]*")))
> 	(setq ovl (org-make-overlay beg end))
> 	(org-overlay-put ovl 'face 'secondary-selection)
> 	(org-overlay-put ovl 'edit-buffer buffer)
> @@ -186,8 +186,7 @@ the edited version."
> 				'(display nil invisible nil intangible nil))
> 	(org-do-remove-indentation)
> 	(let ((org-inhibit-startup t))
> -	  (funcall lang-f)
> -	  (org-src-mode))
> +	  (funcall lang-f))
> 	(set (make-local-variable 'org-edit-src-force-single-line) single)
> 	(set (make-local-variable 'org-edit-src-from-org-mode) org-mode-p)
> 	(when lfmt
> @@ -201,6 +200,7 @@ the edited version."
> 	(org-set-local 'org-edit-src-end-marker end)
> 	(org-set-local 'org-edit-src-overlay ovl)
> 	(org-set-local 'org-edit-src-nindent nindent)
> +	(org-src-mode)
> 	(and org-edit-src-persistent-message
> 	     (org-set-local 'header-line-format msg)))
>       (message "%s" msg)
> @@ -400,12 +400,13 @@ the language, a switch telling of the content  
> should be in a single line."
> (defun org-edit-src-exit ()
>   "Exit special edit and protect problematic lines."
>   (interactive)
> -  (unless (string-match "\\`*Org Edit " (buffer-name (current- 
> buffer)))
> -    (error "This is not an sub-editing buffer, something is  
> wrong..."))
> +  (unless org-edit-src-from-org-mode
> +    (error "This is not a sub-editing buffer, something is  
> wrong..."))
>   (let ((beg org-edit-src-beg-marker)
> 	(end org-edit-src-end-marker)
> 	(ovl org-edit-src-overlay)
> 	(buffer (current-buffer))
> +	(buffer-file-name nil)
> 	(nindent org-edit-src-nindent)
> 	code line)
>     (untabify (point-min) (point-max))
> @@ -444,7 +445,6 @@ the language, a switch telling of the content  
> should be in a single line."
>     (switch-to-buffer (marker-buffer beg))
>     (kill-buffer buffer)
>     (goto-char beg)
> -    (org-delete-overlay ovl)
>     (delete-region beg end)
>     (insert code)
>     (goto-char beg)
> @@ -464,6 +464,18 @@ the language, a switch telling of the content  
> should be in a single line."
>     (goto-char (min p (point-max)))
>     (message (or msg ""))))
>
> +(defun org-src-mode-configure-buffer ()
> +  (setq buffer-offer-save t)
> +  (setq buffer-file-name
> +	(concat (buffer-file-name (marker-buffer org-edit-src-beg-marker))
> +		"[" (buffer-name) "]"))
> +  (set (if (featurep 'xemacs) 'write-contents-hooks 'write-contents- 
> functions)
> +       '(org-edit-src-save))
> +  (org-add-hook 'kill-buffer-hook
> +		'(lambda () (org-delete-overlay org-edit-src-overlay)) nil 'local))
> +
> +(org-add-hook 'org-src-mode-hook 'org-src-mode-configure-buffer)
> +
> (provide 'org-src)
>
> ;; arch-tag: 6a1fc84f-dec7-47be-a416-64be56bea5d8
> --8<---------------cut here---------------end--------------->8---
>
> * Notes on patch
> *** write-contents-functions
>    A good start seems to be to use org-src-mode-hook to add
>    org-edit-src-save to the write-contents-functions list. This
>    means that when it comes to saving, org-edit-src-save will be
>    called and no subsequent attempt will be made to save the buffer
>    in the normal way. (This should obviate the remapping of C-x C-s
>    to org-edit-src-save in org-src.el)
> *** buffer-offer-save
>    We also want to set this to t.
>
> *** Where does this get us?
>
>    - C-x s still does *not* offer to save the edit buffer. That's
>      because buffer-file-name is nil.
>
>    - C-x C-c does ask us whether we want to save the
>      edit buffer. However, since buffer-file-name is nil it asks us
>      for a file name. The check in org-edit-src-exit throws an error
>      unless the buffer is named '* Org Edit '...
>
>    - C-x k kills the buffer silently, leaving a broken overlay
>      link. If buffer-file-name were set, it would have warned that
>      the buffer was modified.
>
> *** buffer-file-name
>    So, that all suggests that we need to set buffer-file-name, even
>    though we don't really want to associate this buffer with a file
>    in the normal way. As for the file name, my current suggestion
>    is parent-org-filename[edit-buffer-name].
>
>    [I had to move the (org-src-mode) call to the end of
>    org-edit-src-code to make sure that the required variables were
>    defined when the hook was called.]
>
> *** And so where are we now?
>    - C-x s *does* offer to save the edit buffer, but in saving
>      produces a warning that the edit buffer is modified.
>    - C-x k now gives a warning that the edit buffer is modified
>      (even if it's not).
>    - C-x C-c is working as desired, except that again we get
>      warnings that the edit buffer is modified, once when we save,
>      and again just before exiting emacs.
>    - And C-c ' now issues a warning that the edit buffer is
>      modified when we leave it, which we don't want.
> *** So, we need to get rid of the buffer modification warnings.
>    I've made buffer-file-name nil inside the let binding in
>    org-edit-src-exit.
> *** And?
>    - C-x s behaves as desired, except that as was already the case,
>      the edit buffer is always considered modified, and so repeated
>      invocations keep saving it.
>    - As was already the case, C-x k always gives a warning that the
>      edit buffer has been modified.
>    - C-x C-c is as desired (offers to save the edit buffer) except
>      that it warns of the modified buffer just before exiting.
>    - C-c ' is as it should be (silent)
>
>
> Carsten Dominik <carsten.dominik@gmail.com> writes:
>
>> Hi Dan,
>>
>> On Jun 2, 2009, at 5:02 PM, Dan Davison wrote:
>>
>>> Following on from the recent improvements to the *Org Edit Src
>>> Example*
>>> buffer, I have one more proposal: I have remapped C-x C-s so that it
>>> saves the code in the org buffer, rather than offering to save the
>>> Edit
>>> buffer itself (as it used to be with the indirect edit buffer). I  
>>> find
>>> this essential, although I recognise that remapping C-x C-s is a
>>> rather
>>> radical thing to do to an emacs buffer.
>>
>>
>> But allowed:  From the Emacs lisp docs, under Major Mode Conventions:
>>
>>     It is legitimate for a major mode to rebind a standard key
>>     sequence if it provides a command that does "the same job" in a
>>     way better suited to the text this mode is used for.
>>
>> I'd say, your's is a perfect example for this rule.
>>
>>
>>> I am using the simple-minded approach below; it seems to work fine  
>>> (I
>>> don't even notice a flicker -- should I be surprised at that?),  
>>> but if
>>> someone can suggest an improved implementation I'd be happy to learn
>>> how
>>> it should be done (perhaps there are buffer variables other than  
>>> point
>>> and mark that I should restore? Is there a general mechanism I  
>>> should
>>> use for this?).
>>>
>>> Dan
>>>
>>> (defun org-edit-src-save ()
>>> "Update the parent org buffer with the edited source code, save
>>> the parent org-buffer, and return to the source code edit
>>> buffer."
>>> (interactive)
>>> (let ((p (point))
>>> 	(m (mark)))
>>>   (org-edit-src-exit)
>>>   (save-buffer)
>>>   (org-edit-src-code)
>>>   (set-mark m)
>>>   (goto-char p)))
>>
>> This is already excellent.  I have changed it only slightly,
>> in order to get a better message when this command finishes, and
>> because `push-mark' should be used here instead of `set-mark'.
>>
>> (defun org-edit-src-save ()
>>  "Save parent buffer with current state source-code buffer."
>>  (interactive)
>>  (let ((p (point)) (m (mark)) msg)
>>    (org-edit-src-exit)
>>    (save-buffer)
>>    (setq msg (current-message))
>>    (org-edit-src-code)
>>    (push-mark m 'nomessage)
>>    (goto-char p)
>>    (message (or msg ""))))
>>
>> I have added your code, thanks.
>>
>> - Carsten

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

* Re: Re: PATCH: proposed improvements to org-src-mode
  2009-08-12 12:22     ` Carsten Dominik
@ 2009-08-12 14:58       ` Dan Davison
  2009-08-19 11:03         ` Dan Davison
  0 siblings, 1 reply; 15+ messages in thread
From: Dan Davison @ 2009-08-12 14:58 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: emacs org-mode mailing list

Carsten Dominik <carsten.dominik@gmail.com> writes:

> Hi Dan,
>
> thank you for studying and describing these issues, and for proposing
> a patch.
>
> I am not sure that the implementation you offer is the cleanest
> possibile, I definitely do not want to attach a file to this temporary
> editing buffer.

Just to be clear, my proposal sets buffer-file-name, but never actually
creates the file. I found that necessary in order to make emacs believe
that the buffer needed saving: although artificial, a non-nil
buffer-file-name (together with buffer-offer-save) has the following
three desirable effects:

1. C-x s offers to save the edit buffer

2. C-x k warns that the buffer is modified

3. C-x C-c doesn't prompt for a file name; it just performs the desired
   save operation (via org-edit-src-save) before exiting 

Another part of the patch is adding org-edit-src-save to the
write-contents-functions list. This means that not only C-x C-s but also
C-x s and C-x C-c automatically use org-edit-save when saving the
buffer.

> It is probably better to install
> a kill-buffer-hook to force the query, for example, or even
> to advise the save-buffers-kill-terminal function to
> handle the special case.
>
> First of all, in reading your mail I have a few problems
> understanding exactly what you mean, because I have the feeling
> that you do not clearly distinguish between `C-x s' and `C-x C-s'.
> Because you write that `C-x s' is bound to `org-edit-src-save'
> which is is not.
>
> Could you please review your post to make sure that you
> are using the correct keys?

I think there was just the one such error:

<...>

>> | C-x s | org-edit-src-save | save the code in the source code block
        ^^^
        C-s

> The I will comment further.

That would be great. Now that I've started looking into this, I'm quite
keen to work out what the correct solution is.

Dan


>> in the parent org file |
>> | C-c ' | org-edit-src-exit | return to the parent org file with new
>> code                   |
>>
>> Furthermore, while the edit buffer is alive, the originating code
>> block
>> is subject to a special overlay which links to the edit buffer when
>> you
>> click on it. This is all excellent, and I use it every day, but I
>> think
>> there's still a couple of improvements that we should make.
>>
>> Specifically, I'm proposing that the following are bugs:
>>
>> * Proposed bug I
>>      C-x k kills the edit buffer without questions; the overlay
>>      remains, but now links to a deleted buffer.
>> * Proposed bug II
>>      C-x C-c kills a modified edit buffer silently, without offering
>> to
>>      save your work. I have lost work like that a number of times
>>      recently.
>> * Proposed bug III
>>      C-x s does not offer to save a modified edit buffer
>>
>> The attached patch does the following.
>> - C-x s offers to save edit buffers
>> - C-x C-c offers to save edit buffers
>> - C-x k warns that you're killing an edit buffer
>> - If you do kill an edit buffer, the overlay in the parent buffer is
>> removed
>> - Edit buffers are named *Org Src <orgbuf>[<lang>]*, where
>>  <orgbuf> is the name of the org-mode buffer containing this
>>  source code block, and lang is the language major mode.
>> - An internal detail is that org-edit-src-save is added to the
>>  write-contents-functions list, which means that it is no longer
>>  necessary to explicitly remap C-x C-s to org-edit-src-save
>>
>> * Notes
>>  This patch gives the desired behaviour, at the cost of being forced
>> to
>>  assign a buffer-file-name to the edit buffer. The consequence is that
>>  the edit buffer is considered to always be modified, since a file of
>>  that name is never actually written to (doesn't even exist). I didn't
>>  manage to come up with a way to trick emacs into holding the
>>  appropriate beliefs about whether the buffer had been modified. But
>> in
>>  any case, I think there's an argument that these modifications
>>  warnings are a good thing, because one should not leave active edit
>>  buffers around: you should always have exited with C-c ' first.
>>
>> Just in case it is helpful, I am including the notes I made in the
>> course of making these changes at the very bottom of the email.
>>
>> Dan
>>
>> p.s. In these two lines:
>> -  (unless (string-match "\\`*Org Edit " (buffer-name (current-
>> buffer)))
>> -    (error "This is not an sub-editing buffer, something is
>> wrong..."))
>> +  (unless org-edit-src-from-org-mode
>> +    (error "This is not a sub-editing buffer, something is
>> wrong..."))
>>
>> I assumed that org-edit-src-from-org-mode was an appropriate test. But
>> that may be incorrect as I am not certain what the intention was for
>> that variable.
>>
>> --8<---------------cut here---------------start------------->8---
>> diff --git a/lisp/org-src.el b/lisp/org-src.el
>> index 2a6c087..a5816d2 100644
>> --- a/lisp/org-src.el
>> +++ b/lisp/org-src.el
>> @@ -113,7 +113,6 @@ but which mess up the display of a snippet in
>> Org exported files.")
>>
>> (defvar org-src-mode-map (make-sparse-keymap))
>> (define-key org-src-mode-map "\C-c'" 'org-edit-src-exit)
>> -(define-key org-src-mode-map "\C-x\C-s" 'org-edit-src-save)
>> (defvar org-edit-src-force-single-line nil)
>> (defvar org-edit-src-from-org-mode nil)
>> (defvar org-edit-src-picture nil)
>> @@ -168,7 +167,8 @@ the edited version."
>> 	    (if (boundp 'org-edit-src-overlay)
>> 		(org-delete-overlay org-edit-src-overlay)))
>> 	  (kill-buffer buffer))
>> -	(setq buffer (generate-new-buffer "*Org Edit Src Example*"))
>> +	(setq buffer (generate-new-buffer
>> +		      (concat "*Org Src " (file-name-nondirectory
>> buffer-file-
>> name) "[" lang "]*")))
>> 	(setq ovl (org-make-overlay beg end))
>> 	(org-overlay-put ovl 'face 'secondary-selection)
>> 	(org-overlay-put ovl 'edit-buffer buffer)
>> @@ -186,8 +186,7 @@ the edited version."
>> 				'(display nil invisible nil intangible nil))
>> 	(org-do-remove-indentation)
>> 	(let ((org-inhibit-startup t))
>> -	  (funcall lang-f)
>> -	  (org-src-mode))
>> +	  (funcall lang-f))
>> 	(set (make-local-variable 'org-edit-src-force-single-line) single)
>> 	(set (make-local-variable 'org-edit-src-from-org-mode) org-mode-p)
>> 	(when lfmt
>> @@ -201,6 +200,7 @@ the edited version."
>> 	(org-set-local 'org-edit-src-end-marker end)
>> 	(org-set-local 'org-edit-src-overlay ovl)
>> 	(org-set-local 'org-edit-src-nindent nindent)
>> +	(org-src-mode)
>> 	(and org-edit-src-persistent-message
>> 	     (org-set-local 'header-line-format msg)))
>>       (message "%s" msg)
>> @@ -400,12 +400,13 @@ the language, a switch telling of the content
>> should be in a single line."
>> (defun org-edit-src-exit ()
>>   "Exit special edit and protect problematic lines."
>>   (interactive)
>> -  (unless (string-match "\\`*Org Edit " (buffer-name (current-
>> buffer)))
>> -    (error "This is not an sub-editing buffer, something is
>> wrong..."))
>> +  (unless org-edit-src-from-org-mode
>> +    (error "This is not a sub-editing buffer, something is
>> wrong..."))
>>   (let ((beg org-edit-src-beg-marker)
>> 	(end org-edit-src-end-marker)
>> 	(ovl org-edit-src-overlay)
>> 	(buffer (current-buffer))
>> +	(buffer-file-name nil)
>> 	(nindent org-edit-src-nindent)
>> 	code line)
>>     (untabify (point-min) (point-max))
>> @@ -444,7 +445,6 @@ the language, a switch telling of the content
>> should be in a single line."
>>     (switch-to-buffer (marker-buffer beg))
>>     (kill-buffer buffer)
>>     (goto-char beg)
>> -    (org-delete-overlay ovl)
>>     (delete-region beg end)
>>     (insert code)
>>     (goto-char beg)
>> @@ -464,6 +464,18 @@ the language, a switch telling of the content
>> should be in a single line."
>>     (goto-char (min p (point-max)))
>>     (message (or msg ""))))
>>
>> +(defun org-src-mode-configure-buffer ()
>> +  (setq buffer-offer-save t)
>> +  (setq buffer-file-name
>> +	(concat (buffer-file-name (marker-buffer org-edit-src-beg-marker))
>> +		"[" (buffer-name) "]"))
>> +  (set (if (featurep 'xemacs) 'write-contents-hooks
>> write-contents-
>> functions)
>> +       '(org-edit-src-save))
>> +  (org-add-hook 'kill-buffer-hook
>> +		'(lambda () (org-delete-overlay org-edit-src-overlay)) nil 'local))
>> +
>> +(org-add-hook 'org-src-mode-hook 'org-src-mode-configure-buffer)
>> +
>> (provide 'org-src)
>>
>> ;; arch-tag: 6a1fc84f-dec7-47be-a416-64be56bea5d8
>> --8<---------------cut here---------------end--------------->8---
>>
>> * Notes on patch
>> *** write-contents-functions
>>    A good start seems to be to use org-src-mode-hook to add
>>    org-edit-src-save to the write-contents-functions list. This
>>    means that when it comes to saving, org-edit-src-save will be
>>    called and no subsequent attempt will be made to save the buffer
>>    in the normal way. (This should obviate the remapping of C-x C-s
>>    to org-edit-src-save in org-src.el)
>> *** buffer-offer-save
>>    We also want to set this to t.
>>
>> *** Where does this get us?
>>
>>    - C-x s still does *not* offer to save the edit buffer. That's
>>      because buffer-file-name is nil.
>>
>>    - C-x C-c does ask us whether we want to save the
>>      edit buffer. However, since buffer-file-name is nil it asks us
>>      for a file name. The check in org-edit-src-exit throws an error
>>      unless the buffer is named '* Org Edit '...
>>
>>    - C-x k kills the buffer silently, leaving a broken overlay
>>      link. If buffer-file-name were set, it would have warned that
>>      the buffer was modified.
>>
>> *** buffer-file-name
>>    So, that all suggests that we need to set buffer-file-name, even
>>    though we don't really want to associate this buffer with a file
>>    in the normal way. As for the file name, my current suggestion
>>    is parent-org-filename[edit-buffer-name].
>>
>>    [I had to move the (org-src-mode) call to the end of
>>    org-edit-src-code to make sure that the required variables were
>>    defined when the hook was called.]
>>
>> *** And so where are we now?
>>    - C-x s *does* offer to save the edit buffer, but in saving
>>      produces a warning that the edit buffer is modified.
>>    - C-x k now gives a warning that the edit buffer is modified
>>      (even if it's not).
>>    - C-x C-c is working as desired, except that again we get
>>      warnings that the edit buffer is modified, once when we save,
>>      and again just before exiting emacs.
>>    - And C-c ' now issues a warning that the edit buffer is
>>      modified when we leave it, which we don't want.
>> *** So, we need to get rid of the buffer modification warnings.
>>    I've made buffer-file-name nil inside the let binding in
>>    org-edit-src-exit.
>> *** And?
>>    - C-x s behaves as desired, except that as was already the case,
>>      the edit buffer is always considered modified, and so repeated
>>      invocations keep saving it.
>>    - As was already the case, C-x k always gives a warning that the
>>      edit buffer has been modified.
>>    - C-x C-c is as desired (offers to save the edit buffer) except
>>      that it warns of the modified buffer just before exiting.
>>    - C-c ' is as it should be (silent)
>>
>>
>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>>
>>> Hi Dan,
>>>
>>> On Jun 2, 2009, at 5:02 PM, Dan Davison wrote:
>>>
>>>> Following on from the recent improvements to the *Org Edit Src
>>>> Example*
>>>> buffer, I have one more proposal: I have remapped C-x C-s so that it
>>>> saves the code in the org buffer, rather than offering to save the
>>>> Edit
>>>> buffer itself (as it used to be with the indirect edit buffer). I
>>>> find
>>>> this essential, although I recognise that remapping C-x C-s is a
>>>> rather
>>>> radical thing to do to an emacs buffer.
>>>
>>>
>>> But allowed:  From the Emacs lisp docs, under Major Mode Conventions:
>>>
>>>     It is legitimate for a major mode to rebind a standard key
>>>     sequence if it provides a command that does "the same job" in a
>>>     way better suited to the text this mode is used for.
>>>
>>> I'd say, your's is a perfect example for this rule.
>>>
>>>
>>>> I am using the simple-minded approach below; it seems to work fine
>>>> (I
>>>> don't even notice a flicker -- should I be surprised at that?),
>>>> but if
>>>> someone can suggest an improved implementation I'd be happy to learn
>>>> how
>>>> it should be done (perhaps there are buffer variables other than
>>>> point
>>>> and mark that I should restore? Is there a general mechanism I
>>>> should
>>>> use for this?).
>>>>
>>>> Dan
>>>>
>>>> (defun org-edit-src-save ()
>>>> "Update the parent org buffer with the edited source code, save
>>>> the parent org-buffer, and return to the source code edit
>>>> buffer."
>>>> (interactive)
>>>> (let ((p (point))
>>>> 	(m (mark)))
>>>>   (org-edit-src-exit)
>>>>   (save-buffer)
>>>>   (org-edit-src-code)
>>>>   (set-mark m)
>>>>   (goto-char p)))
>>>
>>> This is already excellent.  I have changed it only slightly,
>>> in order to get a better message when this command finishes, and
>>> because `push-mark' should be used here instead of `set-mark'.
>>>
>>> (defun org-edit-src-save ()
>>>  "Save parent buffer with current state source-code buffer."
>>>  (interactive)
>>>  (let ((p (point)) (m (mark)) msg)
>>>    (org-edit-src-exit)
>>>    (save-buffer)
>>>    (setq msg (current-message))
>>>    (org-edit-src-code)
>>>    (push-mark m 'nomessage)
>>>    (goto-char p)
>>>    (message (or msg ""))))
>>>
>>> I have added your code, thanks.
>>>
>>> - Carsten
>
>
>
> _______________________________________________
> Emacs-orgmode mailing list
> Remember: use `Reply All' to send replies to the list.
> Emacs-orgmode@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-orgmode

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

* Re: Re: PATCH: proposed improvements to org-src-mode
  2009-08-12 14:58       ` Dan Davison
@ 2009-08-19 11:03         ` Dan Davison
  2009-08-24 12:17           ` Carsten Dominik
  0 siblings, 1 reply; 15+ messages in thread
From: Dan Davison @ 2009-08-19 11:03 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: emacs org-mode mailing list

Dan Davison <davison@stats.ox.ac.uk> writes:

> Carsten Dominik <carsten.dominik@gmail.com> writes:
>
>> Hi Dan,
>>
>> thank you for studying and describing these issues, and for proposing
>> a patch.

I have noticed a bug in the patch I proposed: the configuration of the
edit buffer for saving must be done only after C-c ', and not for
example when entering org-src-mode during HTML export. Here's the
revised patch (again, assuming org-edit-src-from-org-mode is a valid
test that C-c ' has just been done).

Dan

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/org-src.el b/lisp/org-src.el
index 2a6c087..6ba58f5 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -113,7 +113,6 @@ but which mess up the display of a snippet in Org exported files.")
 
 (defvar org-src-mode-map (make-sparse-keymap))
 (define-key org-src-mode-map "\C-c'" 'org-edit-src-exit)
-(define-key org-src-mode-map "\C-x\C-s" 'org-edit-src-save)
 (defvar org-edit-src-force-single-line nil)
 (defvar org-edit-src-from-org-mode nil)
 (defvar org-edit-src-picture nil)
@@ -168,7 +167,8 @@ the edited version."
 	    (if (boundp 'org-edit-src-overlay)
 		(org-delete-overlay org-edit-src-overlay)))
 	  (kill-buffer buffer))
-	(setq buffer (generate-new-buffer "*Org Edit Src Example*"))
+	(setq buffer (generate-new-buffer
+		      (concat "*Org Src " (file-name-nondirectory buffer-file-name) "[" lang "]*")))
 	(setq ovl (org-make-overlay beg end))
 	(org-overlay-put ovl 'face 'secondary-selection)
 	(org-overlay-put ovl 'edit-buffer buffer)
@@ -186,8 +186,7 @@ the edited version."
 				'(display nil invisible nil intangible nil))
 	(org-do-remove-indentation)
 	(let ((org-inhibit-startup t))
-	  (funcall lang-f)
-	  (org-src-mode))
+	  (funcall lang-f))
 	(set (make-local-variable 'org-edit-src-force-single-line) single)
 	(set (make-local-variable 'org-edit-src-from-org-mode) org-mode-p)
 	(when lfmt
@@ -201,6 +200,7 @@ the edited version."
 	(org-set-local 'org-edit-src-end-marker end)
 	(org-set-local 'org-edit-src-overlay ovl)
 	(org-set-local 'org-edit-src-nindent nindent)
+	(org-src-mode)
 	(and org-edit-src-persistent-message
 	     (org-set-local 'header-line-format msg)))
       (message "%s" msg)
@@ -400,12 +400,13 @@ the language, a switch telling of the content should be in a single line."
 (defun org-edit-src-exit ()
   "Exit special edit and protect problematic lines."
   (interactive)
-  (unless (string-match "\\`*Org Edit " (buffer-name (current-buffer)))
-    (error "This is not an sub-editing buffer, something is wrong..."))
+  (unless org-edit-src-from-org-mode
+    (error "This is not a sub-editing buffer, something is wrong..."))
   (let ((beg org-edit-src-beg-marker)
 	(end org-edit-src-end-marker)
 	(ovl org-edit-src-overlay)
 	(buffer (current-buffer))
+	(buffer-file-name nil)
 	(nindent org-edit-src-nindent)
 	code line)
     (untabify (point-min) (point-max))
@@ -444,7 +445,6 @@ the language, a switch telling of the content should be in a single line."
     (switch-to-buffer (marker-buffer beg))
     (kill-buffer buffer)
     (goto-char beg)
-    (org-delete-overlay ovl)
     (delete-region beg end)
     (insert code)
     (goto-char beg)
@@ -464,6 +464,19 @@ the language, a switch telling of the content should be in a single line."
     (goto-char (min p (point-max)))
     (message (or msg ""))))
 
+(defun org-src-mode-configure-edit-buffer ()
+  (when org-edit-src-from-org-mode
+    (setq buffer-offer-save t)
+    (setq buffer-file-name
+	  (concat (buffer-file-name (marker-buffer org-edit-src-beg-marker))
+		  "[" (buffer-name) "]"))
+    (set (if (featurep 'xemacs) 'write-contents-hooks 'write-contents-functions)
+	 '(org-edit-src-save))
+    (org-add-hook 'kill-buffer-hook
+		  '(lambda () (org-delete-overlay org-edit-src-overlay)) nil 'local)))
+
+(org-add-hook 'org-src-mode-hook 'org-src-mode-configure-edit-buffer)
+
 (provide 'org-src)
 
 ;; arch-tag: 6a1fc84f-dec7-47be-a416-64be56bea5d8
--8<---------------cut here---------------end--------------->8---

>>> * Notes on patch
>>> *** write-contents-functions
>>>    A good start seems to be to use org-src-mode-hook to add
>>>    org-edit-src-save to the write-contents-functions list. This
>>>    means that when it comes to saving, org-edit-src-save will be
>>>    called and no subsequent attempt will be made to save the buffer
>>>    in the normal way. (This should obviate the remapping of C-x C-s
>>>    to org-edit-src-save in org-src.el)
>>> *** buffer-offer-save
>>>    We also want to set this to t.
>>>
>>> *** Where does this get us?
>>>
>>>    - C-x s still does *not* offer to save the edit buffer. That's
>>>      because buffer-file-name is nil.
>>>
>>>    - C-x C-c does ask us whether we want to save the
>>>      edit buffer. However, since buffer-file-name is nil it asks us
>>>      for a file name. The check in org-edit-src-exit throws an error
>>>      unless the buffer is named '* Org Edit '...
>>>
>>>    - C-x k kills the buffer silently, leaving a broken overlay
>>>      link. If buffer-file-name were set, it would have warned that
>>>      the buffer was modified.
>>>
>>> *** buffer-file-name
>>>    So, that all suggests that we need to set buffer-file-name, even
>>>    though we don't really want to associate this buffer with a file
>>>    in the normal way. As for the file name, my current suggestion
>>>    is parent-org-filename[edit-buffer-name].
>>>
>>>    [I had to move the (org-src-mode) call to the end of
>>>    org-edit-src-code to make sure that the required variables were
>>>    defined when the hook was called.]
>>>
>>> *** And so where are we now?
>>>    - C-x s *does* offer to save the edit buffer, but in saving
>>>      produces a warning that the edit buffer is modified.
>>>    - C-x k now gives a warning that the edit buffer is modified
>>>      (even if it's not).
>>>    - C-x C-c is working as desired, except that again we get
>>>      warnings that the edit buffer is modified, once when we save,
>>>      and again just before exiting emacs.
>>>    - And C-c ' now issues a warning that the edit buffer is
>>>      modified when we leave it, which we don't want.
>>> *** So, we need to get rid of the buffer modification warnings.
>>>    I've made buffer-file-name nil inside the let binding in
>>>    org-edit-src-exit.
>>> *** And?
>>>    - C-x s behaves as desired, except that as was already the case,
>>>      the edit buffer is always considered modified, and so repeated
>>>      invocations keep saving it.
>>>    - As was already the case, C-x k always gives a warning that the
>>>      edit buffer has been modified.
>>>    - C-x C-c is as desired (offers to save the edit buffer) except
>>>      that it warns of the modified buffer just before exiting.
>>>    - C-c ' is as it should be (silent)

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

* Re: Re: PATCH: proposed improvements to org-src-mode
  2009-08-19 11:03         ` Dan Davison
@ 2009-08-24 12:17           ` Carsten Dominik
  2009-08-28  2:36             ` Dan Davison
  0 siblings, 1 reply; 15+ messages in thread
From: Carsten Dominik @ 2009-08-24 12:17 UTC (permalink / raw)
  To: Dan Davison; +Cc: emacs org-mode mailing list

Hi Dan,

I am now finally looking at your patch.

A few questions:

On Aug 19, 2009, at 1:03 PM, Dan Davison wrote:

> Dan Davison <davison@stats.ox.ac.uk> writes:
>
>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>>
>>> Hi Dan,
>>>
>>> thank you for studying and describing these issues, and for  
>>> proposing
>>> a patch.
>
> I have noticed a bug in the patch I proposed: the configuration of the
> edit buffer for saving must be done only after C-c ', and not for
> example when entering org-src-mode during HTML export. Here's the
> revised patch (again, assuming org-edit-src-from-org-mode is a valid
> test that C-c ' has just been done).
>
> Dan
>
> --8<---------------cut here---------------start------------->8---
> diff --git a/lisp/org-src.el b/lisp/org-src.el
> index 2a6c087..6ba58f5 100644
> --- a/lisp/org-src.el
> +++ b/lisp/org-src.el
> @@ -113,7 +113,6 @@ but which mess up the display of a snippet in  
> Org exported files.")
>
> (defvar org-src-mode-map (make-sparse-keymap))
> (define-key org-src-mode-map "\C-c'" 'org-edit-src-exit)
> -(define-key org-src-mode-map "\C-x\C-s" 'org-edit-src-save)
> (defvar org-edit-src-force-single-line nil)
> (defvar org-edit-src-from-org-mode nil)
> (defvar org-edit-src-picture nil)
> @@ -168,7 +167,8 @@ the edited version."
> 	    (if (boundp 'org-edit-src-overlay)
> 		(org-delete-overlay org-edit-src-overlay)))
> 	  (kill-buffer buffer))
> -	(setq buffer (generate-new-buffer "*Org Edit Src Example*"))
> +	(setq buffer (generate-new-buffer
> +		      (concat "*Org Src " (file-name-nondirectory buffer-file- 
> name) "[" lang "]*")))
> 	(setq ovl (org-make-overlay beg end))
> 	(org-overlay-put ovl 'face 'secondary-selection)
> 	(org-overlay-put ovl 'edit-buffer buffer)
> @@ -186,8 +186,7 @@ the edited version."
> 				'(display nil invisible nil intangible nil))
> 	(org-do-remove-indentation)
> 	(let ((org-inhibit-startup t))
> -	  (funcall lang-f)
> -	  (org-src-mode))

You are moving the call to org-src-mode only so that you have org-edit- 
src-from-org defined in the hook, right?

> +	  (funcall lang-f))
> 	(set (make-local-variable 'org-edit-src-force-single-line) single)
> 	(set (make-local-variable 'org-edit-src-from-org-mode) org-mode-p)
> 	(when lfmt
> @@ -201,6 +200,7 @@ the edited version."
> 	(org-set-local 'org-edit-src-end-marker end)
> 	(org-set-local 'org-edit-src-overlay ovl)
> 	(org-set-local 'org-edit-src-nindent nindent)
> +	(org-src-mode)
> 	(and org-edit-src-persistent-message
> 	     (org-set-local 'header-line-format msg)))
>       (message "%s" msg)
> @@ -400,12 +400,13 @@ the language, a switch telling of the content  
> should be in a single line."
> (defun org-edit-src-exit ()
>   "Exit special edit and protect problematic lines."
>   (interactive)
> -  (unless (string-match "\\`*Org Edit " (buffer-name (current- 
> buffer)))
> -    (error "This is not an sub-editing buffer, something is  
> wrong..."))
> +  (unless org-edit-src-from-org-mode
> +    (error "This is not a sub-editing buffer, something is  
> wrong..."))
>   (let ((beg org-edit-src-beg-marker)
> 	(end org-edit-src-end-marker)
> 	(ovl org-edit-src-overlay)
> 	(buffer (current-buffer))
> +	(buffer-file-name nil)

What is the above line for?

> 	(nindent org-edit-src-nindent)
> 	code line)
>     (untabify (point-min) (point-max))
> @@ -444,7 +445,6 @@ the language, a switch telling of the content  
> should be in a single line."
>     (switch-to-buffer (marker-buffer beg))
>     (kill-buffer buffer)
>     (goto-char beg)
> -    (org-delete-overlay ovl)

Why are you removing this line?

>     (delete-region beg end)
>     (insert code)
>     (goto-char beg)
> @@ -464,6 +464,19 @@ the language, a switch telling of the content  
> should be in a single line."
>     (goto-char (min p (point-max)))
>     (message (or msg ""))))
>
> +(defun org-src-mode-configure-edit-buffer ()
> +  (when org-edit-src-from-org-mode
> +    (setq buffer-offer-save t)
> +    (setq buffer-file-name
> +	  (concat (buffer-file-name (marker-buffer org-edit-src-beg-marker))
> +		  "[" (buffer-name) "]"))
> +    (set (if (featurep 'xemacs) 'write-contents-hooks 'write- 
> contents-functions)
> +	 '(org-edit-src-save))
> +    (org-add-hook 'kill-buffer-hook
> +		  '(lambda () (org-delete-overlay org-edit-src-overlay)) nil  
> 'local)))
> +
> +(org-add-hook 'org-src-mode-hook 'org-src-mode-configure-edit-buffer)
> +
> (provide 'org-src)
>
> ;; arch-tag: 6a1fc84f-dec7-47be-a416-64be56bea5d8
> --8<---------------cut here---------------end--------------->8---


I believe an important addition to your patch would be to

    (set-buffer-modified-p nil)

when entering org-src-mode.  Otherwise, if I exit Emacs and reply "y"  
to all safe-this-buffer questions, then I still get a complaint about  
a buffer with changes....

Thanks!




- Carsten

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

* Re: Re: PATCH: proposed improvements to org-src-mode
  2009-08-24 12:17           ` Carsten Dominik
@ 2009-08-28  2:36             ` Dan Davison
  2009-08-28  7:59               ` Carsten Dominik
  0 siblings, 1 reply; 15+ messages in thread
From: Dan Davison @ 2009-08-28  2:36 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: emacs org-mode mailing list

Carsten Dominik <carsten.dominik@gmail.com> writes:

> Hi Dan,
>
> I am now finally looking at your patch.
>
> A few questions:
>
> On Aug 19, 2009, at 1:03 PM, Dan Davison wrote:
>
>> Dan Davison <davison@stats.ox.ac.uk> writes:
>>
>>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>>>
>>>> Hi Dan,
>>>>
>>>> thank you for studying and describing these issues, and for
>>>> proposing
>>>> a patch.
>>
>> I have noticed a bug in the patch I proposed: the configuration of the
>> edit buffer for saving must be done only after C-c ', and not for
>> example when entering org-src-mode during HTML export. Here's the
>> revised patch (again, assuming org-edit-src-from-org-mode is a valid
>> test that C-c ' has just been done).
>>
>> Dan
>>
>> --8<---------------cut here---------------start------------->8---
>> diff --git a/lisp/org-src.el b/lisp/org-src.el
>> index 2a6c087..6ba58f5 100644
>> --- a/lisp/org-src.el
>> +++ b/lisp/org-src.el
>> @@ -113,7 +113,6 @@ but which mess up the display of a snippet in
>> Org exported files.")
>>
>> (defvar org-src-mode-map (make-sparse-keymap))
>> (define-key org-src-mode-map "\C-c'" 'org-edit-src-exit)
>> -(define-key org-src-mode-map "\C-x\C-s" 'org-edit-src-save)
>> (defvar org-edit-src-force-single-line nil)
>> (defvar org-edit-src-from-org-mode nil)
>> (defvar org-edit-src-picture nil)
>> @@ -168,7 +167,8 @@ the edited version."
>> 	    (if (boundp 'org-edit-src-overlay)
>> 		(org-delete-overlay org-edit-src-overlay)))
>> 	  (kill-buffer buffer))
>> -	(setq buffer (generate-new-buffer "*Org Edit Src Example*"))
>> +	(setq buffer (generate-new-buffer
>> +		      (concat "*Org Src " (file-name-nondirectory
>> buffer-file-
>> name) "[" lang "]*")))
>> 	(setq ovl (org-make-overlay beg end))
>> 	(org-overlay-put ovl 'face 'secondary-selection)
>> 	(org-overlay-put ovl 'edit-buffer buffer)
>> @@ -186,8 +186,7 @@ the edited version."
>> 				'(display nil invisible nil intangible nil))
>> 	(org-do-remove-indentation)
>> 	(let ((org-inhibit-startup t))
>> -	  (funcall lang-f)
>> -	  (org-src-mode))
>
> You are moving the call to org-src-mode only so that you have
> org-edit-
> src-from-org defined in the hook, right?

I also use the value of org-edit-src-beg-marker in the hook.

>
>> +	  (funcall lang-f))
>> 	(set (make-local-variable 'org-edit-src-force-single-line) single)
>> 	(set (make-local-variable 'org-edit-src-from-org-mode) org-mode-p)
>> 	(when lfmt
>> @@ -201,6 +200,7 @@ the edited version."
>> 	(org-set-local 'org-edit-src-end-marker end)
>> 	(org-set-local 'org-edit-src-overlay ovl)
>> 	(org-set-local 'org-edit-src-nindent nindent)
>> +	(org-src-mode)
>> 	(and org-edit-src-persistent-message
>> 	     (org-set-local 'header-line-format msg)))
>>       (message "%s" msg)
>> @@ -400,12 +400,13 @@ the language, a switch telling of the content
>> should be in a single line."
>> (defun org-edit-src-exit ()
>>   "Exit special edit and protect problematic lines."
>>   (interactive)
>> -  (unless (string-match "\\`*Org Edit " (buffer-name (current-
>> buffer)))
>> -    (error "This is not an sub-editing buffer, something is
>> wrong..."))
>> +  (unless org-edit-src-from-org-mode
>> +    (error "This is not a sub-editing buffer, something is
>> wrong..."))
>>   (let ((beg org-edit-src-beg-marker)
>> 	(end org-edit-src-end-marker)
>> 	(ovl org-edit-src-overlay)
>> 	(buffer (current-buffer))
>> +	(buffer-file-name nil)
>
> What is the above line for?

I did this so that emacs allows us to kill a modified buffer without
warning (Since, in the case of the src edit buffer, saving it involves
killing it). It seems that if emacs believes a buffer has no associated
file it doesn't warn about modifications. However I think you've pointed
to a less hackish way of doing this below. So instead of setting
buffer-file-name to nil in this line that you are querying, perhaps we
should alter the end of org-edit-src-exit as follows:

    ...
    (setq code (buffer-string))
+   (set-buffer-modified-p nil)
    (switch-to-buffer (marker-buffer beg))
    (kill-buffer buffer)
    (goto-char beg)
    (delete-region beg end)
    (insert code)
    ...

? This would be in addition to your suggestion of (set-buffer-modified-p
nil) when entering org-src-mode.

>
>> 	(nindent org-edit-src-nindent)
>> 	code line)
>>     (untabify (point-min) (point-max))
>> @@ -444,7 +445,6 @@ the language, a switch telling of the content
>> should be in a single line."
>>     (switch-to-buffer (marker-buffer beg))
>>     (kill-buffer buffer)
>>     (goto-char beg)
>> -    (org-delete-overlay ovl)
>
> Why are you removing this line?

The overlay should have been deleted already by the kill-buffer-hook.

>
>>     (delete-region beg end)
>>     (insert code)
>>     (goto-char beg)
>> @@ -464,6 +464,19 @@ the language, a switch telling of the content
>> should be in a single line."
>>     (goto-char (min p (point-max)))
>>     (message (or msg ""))))
>>
>> +(defun org-src-mode-configure-edit-buffer ()
>> +  (when org-edit-src-from-org-mode
>> +    (setq buffer-offer-save t)
>> +    (setq buffer-file-name
>> +	  (concat (buffer-file-name (marker-buffer org-edit-src-beg-marker))
>> +		  "[" (buffer-name) "]"))
>> +    (set (if (featurep 'xemacs) 'write-contents-hooks 'write-
>> contents-functions)
>> +	 '(org-edit-src-save))
>> +    (org-add-hook 'kill-buffer-hook
>> +		  '(lambda () (org-delete-overlay
>> org-edit-src-overlay)) nil 'local)))
>> +
>> +(org-add-hook 'org-src-mode-hook 'org-src-mode-configure-edit-buffer)
>> +
>> (provide 'org-src)
>>
>> ;; arch-tag: 6a1fc84f-dec7-47be-a416-64be56bea5d8
>> --8<---------------cut here---------------end--------------->8---
>
>
> I believe an important addition to your patch would be to
>
>    (set-buffer-modified-p nil)t
>
> when entering org-src-mode.  Otherwise, if I exit Emacs and reply "y"
> to all safe-this-buffer questions, then I still get a complaint about
> a buffer with changes....

Yes, I think I agree (see above).

Dan

>
> Thanks!
>
>
>
>
> - Carsten
>
>
>
> _______________________________________________
> Emacs-orgmode mailing list
> Remember: use `Reply All' to send replies to the list.
> Emacs-orgmode@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-orgmode

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

* Re: Re: PATCH: proposed improvements to org-src-mode
  2009-08-28  2:36             ` Dan Davison
@ 2009-08-28  7:59               ` Carsten Dominik
  2009-09-05 17:33                 ` Dan Davison
  0 siblings, 1 reply; 15+ messages in thread
From: Carsten Dominik @ 2009-08-28  7:59 UTC (permalink / raw)
  To: Dan Davison; +Cc: emacs org-mode mailing list

Hi Dan,

OK, I have now applied the patch.  If you don't mind, could
you please double-check the commit?

Thanks.

- Carsten

On Aug 28, 2009, at 4:36 AM, Dan Davison wrote:

> Carsten Dominik <carsten.dominik@gmail.com> writes:
>
>> Hi Dan,
>>
>> I am now finally looking at your patch.
>>
>> A few questions:
>>
>> On Aug 19, 2009, at 1:03 PM, Dan Davison wrote:
>>
>>> Dan Davison <davison@stats.ox.ac.uk> writes:
>>>
>>>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>>>>
>>>>> Hi Dan,
>>>>>
>>>>> thank you for studying and describing these issues, and for
>>>>> proposing
>>>>> a patch.
>>>
>>> I have noticed a bug in the patch I proposed: the configuration of  
>>> the
>>> edit buffer for saving must be done only after C-c ', and not for
>>> example when entering org-src-mode during HTML export. Here's the
>>> revised patch (again, assuming org-edit-src-from-org-mode is a valid
>>> test that C-c ' has just been done).
>>>
>>> Dan
>>>
>>> --8<---------------cut here---------------start------------->8---
>>> diff --git a/lisp/org-src.el b/lisp/org-src.el
>>> index 2a6c087..6ba58f5 100644
>>> --- a/lisp/org-src.el
>>> +++ b/lisp/org-src.el
>>> @@ -113,7 +113,6 @@ but which mess up the display of a snippet in
>>> Org exported files.")
>>>
>>> (defvar org-src-mode-map (make-sparse-keymap))
>>> (define-key org-src-mode-map "\C-c'" 'org-edit-src-exit)
>>> -(define-key org-src-mode-map "\C-x\C-s" 'org-edit-src-save)
>>> (defvar org-edit-src-force-single-line nil)
>>> (defvar org-edit-src-from-org-mode nil)
>>> (defvar org-edit-src-picture nil)
>>> @@ -168,7 +167,8 @@ the edited version."
>>> 	    (if (boundp 'org-edit-src-overlay)
>>> 		(org-delete-overlay org-edit-src-overlay)))
>>> 	  (kill-buffer buffer))
>>> -	(setq buffer (generate-new-buffer "*Org Edit Src Example*"))
>>> +	(setq buffer (generate-new-buffer
>>> +		      (concat "*Org Src " (file-name-nondirectory
>>> buffer-file-
>>> name) "[" lang "]*")))
>>> 	(setq ovl (org-make-overlay beg end))
>>> 	(org-overlay-put ovl 'face 'secondary-selection)
>>> 	(org-overlay-put ovl 'edit-buffer buffer)
>>> @@ -186,8 +186,7 @@ the edited version."
>>> 				'(display nil invisible nil intangible nil))
>>> 	(org-do-remove-indentation)
>>> 	(let ((org-inhibit-startup t))
>>> -	  (funcall lang-f)
>>> -	  (org-src-mode))
>>
>> You are moving the call to org-src-mode only so that you have
>> org-edit-
>> src-from-org defined in the hook, right?
>
> I also use the value of org-edit-src-beg-marker in the hook.
>
>>
>>> +	  (funcall lang-f))
>>> 	(set (make-local-variable 'org-edit-src-force-single-line) single)
>>> 	(set (make-local-variable 'org-edit-src-from-org-mode) org-mode-p)
>>> 	(when lfmt
>>> @@ -201,6 +200,7 @@ the edited version."
>>> 	(org-set-local 'org-edit-src-end-marker end)
>>> 	(org-set-local 'org-edit-src-overlay ovl)
>>> 	(org-set-local 'org-edit-src-nindent nindent)
>>> +	(org-src-mode)
>>> 	(and org-edit-src-persistent-message
>>> 	     (org-set-local 'header-line-format msg)))
>>>      (message "%s" msg)
>>> @@ -400,12 +400,13 @@ the language, a switch telling of the content
>>> should be in a single line."
>>> (defun org-edit-src-exit ()
>>>  "Exit special edit and protect problematic lines."
>>>  (interactive)
>>> -  (unless (string-match "\\`*Org Edit " (buffer-name (current-
>>> buffer)))
>>> -    (error "This is not an sub-editing buffer, something is
>>> wrong..."))
>>> +  (unless org-edit-src-from-org-mode
>>> +    (error "This is not a sub-editing buffer, something is
>>> wrong..."))
>>>  (let ((beg org-edit-src-beg-marker)
>>> 	(end org-edit-src-end-marker)
>>> 	(ovl org-edit-src-overlay)
>>> 	(buffer (current-buffer))
>>> +	(buffer-file-name nil)
>>
>> What is the above line for?
>
> I did this so that emacs allows us to kill a modified buffer without
> warning (Since, in the case of the src edit buffer, saving it involves
> killing it). It seems that if emacs believes a buffer has no  
> associated
> file it doesn't warn about modifications. However I think you've  
> pointed
> to a less hackish way of doing this below. So instead of setting
> buffer-file-name to nil in this line that you are querying, perhaps we
> should alter the end of org-edit-src-exit as follows:
>
>    ...
>    (setq code (buffer-string))
> +   (set-buffer-modified-p nil)
>    (switch-to-buffer (marker-buffer beg))
>    (kill-buffer buffer)
>    (goto-char beg)
>    (delete-region beg end)
>    (insert code)
>    ...
>
> ? This would be in addition to your suggestion of (set-buffer- 
> modified-p
> nil) when entering org-src-mode.
>
>>
>>> 	(nindent org-edit-src-nindent)
>>> 	code line)
>>>    (untabify (point-min) (point-max))
>>> @@ -444,7 +445,6 @@ the language, a switch telling of the content
>>> should be in a single line."
>>>    (switch-to-buffer (marker-buffer beg))
>>>    (kill-buffer buffer)
>>>    (goto-char beg)
>>> -    (org-delete-overlay ovl)
>>
>> Why are you removing this line?
>
> The overlay should have been deleted already by the kill-buffer-hook.
>
>>
>>>    (delete-region beg end)
>>>    (insert code)
>>>    (goto-char beg)
>>> @@ -464,6 +464,19 @@ the language, a switch telling of the content
>>> should be in a single line."
>>>    (goto-char (min p (point-max)))
>>>    (message (or msg ""))))
>>>
>>> +(defun org-src-mode-configure-edit-buffer ()
>>> +  (when org-edit-src-from-org-mode
>>> +    (setq buffer-offer-save t)
>>> +    (setq buffer-file-name
>>> +	  (concat (buffer-file-name (marker-buffer org-edit-src-beg- 
>>> marker))
>>> +		  "[" (buffer-name) "]"))
>>> +    (set (if (featurep 'xemacs) 'write-contents-hooks 'write-
>>> contents-functions)
>>> +	 '(org-edit-src-save))
>>> +    (org-add-hook 'kill-buffer-hook
>>> +		  '(lambda () (org-delete-overlay
>>> org-edit-src-overlay)) nil 'local)))
>>> +
>>> +(org-add-hook 'org-src-mode-hook 'org-src-mode-configure-edit- 
>>> buffer)
>>> +
>>> (provide 'org-src)
>>>
>>> ;; arch-tag: 6a1fc84f-dec7-47be-a416-64be56bea5d8
>>> --8<---------------cut here---------------end--------------->8---
>>
>>
>> I believe an important addition to your patch would be to
>>
>>   (set-buffer-modified-p nil)t
>>
>> when entering org-src-mode.  Otherwise, if I exit Emacs and reply "y"
>> to all safe-this-buffer questions, then I still get a complaint about
>> a buffer with changes....
>
> Yes, I think I agree (see above).
>
> Dan
>
>>
>> Thanks!
>>
>>
>>
>>
>> - Carsten
>>
>>
>>
>> _______________________________________________
>> Emacs-orgmode mailing list
>> Remember: use `Reply All' to send replies to the list.
>> Emacs-orgmode@gnu.org
>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode

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

* Re: Re: PATCH: proposed improvements to org-src-mode
  2009-08-28  7:59               ` Carsten Dominik
@ 2009-09-05 17:33                 ` Dan Davison
  2009-09-06 11:41                   ` Carsten Dominik
  0 siblings, 1 reply; 15+ messages in thread
From: Dan Davison @ 2009-09-05 17:33 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: emacs org-mode mailing list

Carsten Dominik <carsten.dominik@gmail.com> writes:

> Hi Dan,
>
> OK, I have now applied the patch.  If you don't mind, could
> you please double-check the commit?

[Re.: commit 4b6988bf36cb458c9d113ee4332e016990c1eb04 ]

Thanks Carsten, I looked over the commit and it looked good. However
after using it a bit over the last week I have discovered two bugs and a
code issue. These follow below, together with proposed patches. There
are 3 separate patches. Just in case you want to accept all 3 of them,
I've created a single patch against current git right at the bottom of
the email.

But first, there are a couple of aesthetic choices being made here that
others might have better ideas for than the ones I've come up with so
far. Firstly, how should we name edit buffers? At the moment we're using
my proposal of (actually I just added spaces inside the square brackets)

(concat "*Org Src " org-buffer-name "[ " lang " ]*")

e.g.

"*Org Src testing.org[ python ]*"

Secondly, how shall we name edit buffers for Fixed Width / ASCII drawing
regions? The patch below uses

(concat "*Org Src " org-buffer-name "[ " "Fixed Width" " ]*")

e.g.

"*Org Src testing.org[ Fixed Width ]*"

(Maybe "Literal" would be better? Neither seems perfect.)

Now for the bugs & patches.

* Bug 1: C-x s switches active buffer
  1. open two edit buffers simultaneously and modify them
  2. now issue C-x s in one of the edit buffers
  3. when it comes to saving the second edit buffer, it will steal focus.

Would an appropriate solution be to wrap the body of org-edit-src-save
in a save-window-excursion, as in this patch? I've tried it quickly
and it seems to have the desired effect.

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/org-src.el b/lisp/org-src.el
index fbf395f..df09f39 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -474,14 +474,15 @@ the language, a switch telling of the content should be in a single line."
 (defun org-edit-src-save ()
   "Save parent buffer with current state source-code buffer."
   (interactive)
-  (let ((p (point)) (m (mark)) msg)
-    (org-edit-src-exit)
-    (save-buffer)
-    (setq msg (current-message))
-    (org-edit-src-code)
-    (push-mark m 'nomessage)
-    (goto-char (min p (point-max)))
-    (message (or msg ""))))
+  (save-window-excursion
+    (let ((p (point)) (m (mark)) msg)
+      (org-edit-src-exit)
+      (save-buffer)
+      (setq msg (current-message))
+      (org-edit-src-code)
+      (push-mark m 'nomessage)
+      (goto-char (min p (point-max)))
+      (message (or msg "")))))
--8<---------------cut here---------------end--------------->8---


* Bug 2: org-edit-src-find-buffer fails to find buffer
  - Use C-c ' to switch to an edit buffer and make a modification
  - Return to the org buffer without killing edit buffer
  - Try C-c ' on the same code block
  - You are now in a new edit buffer instead of returning to the original one!

  This is because it had not been updated to reflect the new edit buffer
  naming scheme. I propose that we ensure consistency by creating a new
  function org-src-construct-edit-buffer-name which takes the parent
  buffer name and the language and generates the edit buffer name, as in
  this patch.

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/org-src.el b/lisp/org-src.el
index df09f39..b0932c1 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -185,7 +185,8 @@ the edited version."
                (org-delete-overlay org-edit-src-overlay)))
          (kill-buffer buffer))
        (setq buffer (generate-new-buffer
-                     (concat "*Org Src " (file-name-nondirectory buffer-file-name) "[" lang "]*")))
+                     (org-src-construct-edit-buffer-name
+                      (file-name-nondirectory buffer-file-name) lang)))
        (setq ovl (org-make-overlay beg end))
        (org-overlay-put ovl 'face 'secondary-selection)
        (org-overlay-put ovl 'edit-buffer buffer)
@@ -231,13 +232,17 @@ the edited version."
     (if buf (switch-to-buffer buf)
       (error "Something is wrong here"))))
 
+(defun org-src-construct-edit-buffer-name (org-buffer-name lang)
+  "Construct the buffer name for a source editing buffer"
+  (concat "*Org Src " org-buffer-name "[ " lang " ]*"))
+
 (defun org-edit-src-find-buffer (beg end)
   "Find a source editing buffer that is already editing the region BEG to END."
   (catch 'exit
     (mapc
      (lambda (b)
        (with-current-buffer b
-        (if (and (string-match "\\`*Org Edit " (buffer-name))
+        (if (and (string-match "\\`*Org Src " (buffer-name))
                  (local-variable-p 'org-edit-src-beg-marker (current-buffer))
                  (local-variable-p 'org-edit-src-end-marker (current-buffer))
                  (equal beg org-edit-src-beg-marker)
@@ -289,7 +294,9 @@ the fragment in the Org-mode buffer."
            (if (boundp 'org-edit-src-overlay)
                (org-delete-overlay org-edit-src-overlay)))
          (kill-buffer buffer))
-       (setq buffer (generate-new-buffer "*Org Edit Src Example*"))
+       (setq buffer (generate-new-buffer
+                     (org-src-construct-edit-buffer-name
+                      (file-name-nondirectory buffer-file-name) "Fixed Width")))
        (setq ovl (org-make-overlay beg end))
        (org-overlay-put ovl 'face 'secondary-selection)
        (org-overlay-put ovl 'edit-buffer buffer)
--8<---------------cut here---------------end--------------->8---

* Code issue:
  I can't work out why I was using (file-name-nondirectory
  buffer-file-name) in order to construct the edit buffer name instead
  of (buffer-name). Unless someone can think of a reason for this,
  please apply the following patch.

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/org-src.el b/lisp/org-src.el
index b0932c1..5f21dfd 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -185,8 +185,7 @@ the edited version."
                (org-delete-overlay org-edit-src-overlay)))
          (kill-buffer buffer))
        (setq buffer (generate-new-buffer
-                     (org-src-construct-edit-buffer-name
-                      (file-name-nondirectory buffer-file-name) lang)))
+                     (org-src-construct-edit-buffer-name (buffer-name) lang)))
        (setq ovl (org-make-overlay beg end))
        (org-overlay-put ovl 'face 'secondary-selection)
        (org-overlay-put ovl 'edit-buffer buffer)
@@ -295,8 +294,7 @@ the fragment in the Org-mode buffer."
                (org-delete-overlay org-edit-src-overlay)))
          (kill-buffer buffer))
        (setq buffer (generate-new-buffer
-                     (org-src-construct-edit-buffer-name
-                      (file-name-nondirectory buffer-file-name) "Fixed Width")))
+                     (org-src-construct-edit-buffer-name (buffer-name) "Fixed Width")))
        (setq ovl (org-make-overlay beg end))
        (org-overlay-put ovl 'face 'secondary-selection)
        (org-overlay-put ovl 'edit-buffer buffer)
--8<---------------cut here---------------end--------------->8---


* Single unified patch

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/org-src.el b/lisp/org-src.el
index fbf395f..5f21dfd 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -185,7 +185,7 @@ the edited version."
 		(org-delete-overlay org-edit-src-overlay)))
 	  (kill-buffer buffer))
 	(setq buffer (generate-new-buffer
-		      (concat "*Org Src " (file-name-nondirectory buffer-file-name) "[" lang "]*")))
+		      (org-src-construct-edit-buffer-name (buffer-name) lang)))
 	(setq ovl (org-make-overlay beg end))
 	(org-overlay-put ovl 'face 'secondary-selection)
 	(org-overlay-put ovl 'edit-buffer buffer)
@@ -231,13 +231,17 @@ the edited version."
     (if buf (switch-to-buffer buf)
       (error "Something is wrong here"))))
 
+(defun org-src-construct-edit-buffer-name (org-buffer-name lang)
+  "Construct the buffer name for a source editing buffer"
+  (concat "*Org Src " org-buffer-name "[ " lang " ]*"))
+
 (defun org-edit-src-find-buffer (beg end)
   "Find a source editing buffer that is already editing the region BEG to END."
   (catch 'exit
     (mapc
      (lambda (b)
        (with-current-buffer b
-	 (if (and (string-match "\\`*Org Edit " (buffer-name))
+	 (if (and (string-match "\\`*Org Src " (buffer-name))
 		  (local-variable-p 'org-edit-src-beg-marker (current-buffer))
 		  (local-variable-p 'org-edit-src-end-marker (current-buffer))
 		  (equal beg org-edit-src-beg-marker)
@@ -289,7 +293,8 @@ the fragment in the Org-mode buffer."
 	    (if (boundp 'org-edit-src-overlay)
 		(org-delete-overlay org-edit-src-overlay)))
 	  (kill-buffer buffer))
-	(setq buffer (generate-new-buffer "*Org Edit Src Example*"))
+	(setq buffer (generate-new-buffer
+		      (org-src-construct-edit-buffer-name (buffer-name) "Fixed Width")))
 	(setq ovl (org-make-overlay beg end))
 	(org-overlay-put ovl 'face 'secondary-selection)
 	(org-overlay-put ovl 'edit-buffer buffer)
@@ -474,14 +479,15 @@ the language, a switch telling of the content should be in a single line."
 (defun org-edit-src-save ()
   "Save parent buffer with current state source-code buffer."
   (interactive)
-  (let ((p (point)) (m (mark)) msg)
-    (org-edit-src-exit)
-    (save-buffer)
-    (setq msg (current-message))
-    (org-edit-src-code)
-    (push-mark m 'nomessage)
-    (goto-char (min p (point-max)))
-    (message (or msg ""))))
+  (save-window-excursion
+    (let ((p (point)) (m (mark)) msg)
+      (org-edit-src-exit)
+      (save-buffer)
+      (setq msg (current-message))
+      (org-edit-src-code)
+      (push-mark m 'nomessage)
+      (goto-char (min p (point-max)))
+      (message (or msg "")))))
 
 (defun org-src-mode-configure-edit-buffer ()
   (when org-edit-src-from-org-mode
--8<---------------cut here---------------end--------------->8---


Dan


>
> Thanks.
>
> - Carsten
>
> On Aug 28, 2009, at 4:36 AM, Dan Davison wrote:
>
>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>>
>>> Hi Dan,
>>>
>>> I am now finally looking at your patch.
>>>
>>> A few questions:
>>>
>>> On Aug 19, 2009, at 1:03 PM, Dan Davison wrote:
>>>
>>>> Dan Davison <davison@stats.ox.ac.uk> writes:
>>>>
>>>>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>>>>>
>>>>>> Hi Dan,
>>>>>>
>>>>>> thank you for studying and describing these issues, and for
>>>>>> proposing
>>>>>> a patch.
>>>>
>>>> I have noticed a bug in the patch I proposed: the configuration of
>>>> the
>>>> edit buffer for saving must be done only after C-c ', and not for
>>>> example when entering org-src-mode during HTML export. Here's the
>>>> revised patch (again, assuming org-edit-src-from-org-mode is a valid
>>>> test that C-c ' has just been done).
>>>>
>>>> Dan
>>>>
>>>> --8<---------------cut here---------------start------------->8---
>>>> diff --git a/lisp/org-src.el b/lisp/org-src.el
>>>> index 2a6c087..6ba58f5 100644
>>>> --- a/lisp/org-src.el
>>>> +++ b/lisp/org-src.el
>>>> @@ -113,7 +113,6 @@ but which mess up the display of a snippet in
>>>> Org exported files.")
>>>>
>>>> (defvar org-src-mode-map (make-sparse-keymap))
>>>> (define-key org-src-mode-map "\C-c'" 'org-edit-src-exit)
>>>> -(define-key org-src-mode-map "\C-x\C-s" 'org-edit-src-save)
>>>> (defvar org-edit-src-force-single-line nil)
>>>> (defvar org-edit-src-from-org-mode nil)
>>>> (defvar org-edit-src-picture nil)
>>>> @@ -168,7 +167,8 @@ the edited version."
>>>> 	    (if (boundp 'org-edit-src-overlay)
>>>> 		(org-delete-overlay org-edit-src-overlay)))
>>>> 	  (kill-buffer buffer))
>>>> -	(setq buffer (generate-new-buffer "*Org Edit Src Example*"))
>>>> +	(setq buffer (generate-new-buffer
>>>> +		      (concat "*Org Src " (file-name-nondirectory
>>>> buffer-file-
>>>> name) "[" lang "]*")))
>>>> 	(setq ovl (org-make-overlay beg end))
>>>> 	(org-overlay-put ovl 'face 'secondary-selection)
>>>> 	(org-overlay-put ovl 'edit-buffer buffer)
>>>> @@ -186,8 +186,7 @@ the edited version."
>>>> 				'(display nil invisible nil intangible nil))
>>>> 	(org-do-remove-indentation)
>>>> 	(let ((org-inhibit-startup t))
>>>> -	  (funcall lang-f)
>>>> -	  (org-src-mode))
>>>
>>> You are moving the call to org-src-mode only so that you have
>>> org-edit-
>>> src-from-org defined in the hook, right?
>>
>> I also use the value of org-edit-src-beg-marker in the hook.
>>
>>>
>>>> +	  (funcall lang-f))
>>>> 	(set (make-local-variable 'org-edit-src-force-single-line) single)
>>>> 	(set (make-local-variable 'org-edit-src-from-org-mode) org-mode-p)
>>>> 	(when lfmt
>>>> @@ -201,6 +200,7 @@ the edited version."
>>>> 	(org-set-local 'org-edit-src-end-marker end)
>>>> 	(org-set-local 'org-edit-src-overlay ovl)
>>>> 	(org-set-local 'org-edit-src-nindent nindent)
>>>> +	(org-src-mode)
>>>> 	(and org-edit-src-persistent-message
>>>> 	     (org-set-local 'header-line-format msg)))
>>>>      (message "%s" msg)
>>>> @@ -400,12 +400,13 @@ the language, a switch telling of the content
>>>> should be in a single line."
>>>> (defun org-edit-src-exit ()
>>>>  "Exit special edit and protect problematic lines."
>>>>  (interactive)
>>>> -  (unless (string-match "\\`*Org Edit " (buffer-name (current-
>>>> buffer)))
>>>> -    (error "This is not an sub-editing buffer, something is
>>>> wrong..."))
>>>> +  (unless org-edit-src-from-org-mode
>>>> +    (error "This is not a sub-editing buffer, something is
>>>> wrong..."))
>>>>  (let ((beg org-edit-src-beg-marker)
>>>> 	(end org-edit-src-end-marker)
>>>> 	(ovl org-edit-src-overlay)
>>>> 	(buffer (current-buffer))
>>>> +	(buffer-file-name nil)
>>>
>>> What is the above line for?
>>
>> I did this so that emacs allows us to kill a modified buffer without
>> warning (Since, in the case of the src edit buffer, saving it involves
>> killing it). It seems that if emacs believes a buffer has no
>> associated
>> file it doesn't warn about modifications. However I think you've
>> pointed
>> to a less hackish way of doing this below. So instead of setting
>> buffer-file-name to nil in this line that you are querying, perhaps we
>> should alter the end of org-edit-src-exit as follows:
>>
>>    ...
>>    (setq code (buffer-string))
>> +   (set-buffer-modified-p nil)
>>    (switch-to-buffer (marker-buffer beg))
>>    (kill-buffer buffer)
>>    (goto-char beg)
>>    (delete-region beg end)
>>    (insert code)
>>    ...
>>
>> ? This would be in addition to your suggestion of (set-buffer-
>> modified-p
>> nil) when entering org-src-mode.
>>
>>>
>>>> 	(nindent org-edit-src-nindent)
>>>> 	code line)
>>>>    (untabify (point-min) (point-max))
>>>> @@ -444,7 +445,6 @@ the language, a switch telling of the content
>>>> should be in a single line."
>>>>    (switch-to-buffer (marker-buffer beg))
>>>>    (kill-buffer buffer)
>>>>    (goto-char beg)
>>>> -    (org-delete-overlay ovl)
>>>
>>> Why are you removing this line?
>>
>> The overlay should have been deleted already by the kill-buffer-hook.
>>
>>>
>>>>    (delete-region beg end)
>>>>    (insert code)
>>>>    (goto-char beg)
>>>> @@ -464,6 +464,19 @@ the language, a switch telling of the content
>>>> should be in a single line."
>>>>    (goto-char (min p (point-max)))
>>>>    (message (or msg ""))))
>>>>
>>>> +(defun org-src-mode-configure-edit-buffer ()
>>>> +  (when org-edit-src-from-org-mode
>>>> +    (setq buffer-offer-save t)
>>>> +    (setq buffer-file-name
>>>> +	  (concat (buffer-file-name (marker-buffer org-edit-src-beg-
>>>> marker))
>>>> +		  "[" (buffer-name) "]"))
>>>> +    (set (if (featurep 'xemacs) 'write-contents-hooks 'write-
>>>> contents-functions)
>>>> +	 '(org-edit-src-save))
>>>> +    (org-add-hook 'kill-buffer-hook
>>>> +		  '(lambda () (org-delete-overlay
>>>> org-edit-src-overlay)) nil 'local)))
>>>> +
>>>> +(org-add-hook 'org-src-mode-hook 'org-src-mode-configure-edit-
>>>> buffer)
>>>> +
>>>> (provide 'org-src)
>>>>
>>>> ;; arch-tag: 6a1fc84f-dec7-47be-a416-64be56bea5d8
>>>> --8<---------------cut here---------------end--------------->8---
>>>
>>>
>>> I believe an important addition to your patch would be to
>>>
>>>   (set-buffer-modified-p nil)t
>>>
>>> when entering org-src-mode.  Otherwise, if I exit Emacs and reply "y"
>>> to all safe-this-buffer questions, then I still get a complaint about
>>> a buffer with changes....
>>
>> Yes, I think I agree (see above).
>>
>> Dan
>>
>>>
>>> Thanks!
>>>
>>>
>>>
>>>
>>> - Carsten
>>>
>>>
>>>
>>> _______________________________________________
>>> Emacs-orgmode mailing list
>>> Remember: use `Reply All' to send replies to the list.
>>> Emacs-orgmode@gnu.org
>>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode
>
>
>
> _______________________________________________
> Emacs-orgmode mailing list
> Remember: use `Reply All' to send replies to the list.
> Emacs-orgmode@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-orgmode

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

* Re: Re: PATCH: proposed improvements to org-src-mode
  2009-09-05 17:33                 ` Dan Davison
@ 2009-09-06 11:41                   ` Carsten Dominik
  0 siblings, 0 replies; 15+ messages in thread
From: Carsten Dominik @ 2009-09-06 11:41 UTC (permalink / raw)
  To: Dan Davison; +Cc: emacs org-mode mailing list

Hi Dan,

thanks for doing such a careful job.  I applied all three parts.

- Carsten

On Sep 5, 2009, at 7:33 PM, Dan Davison wrote:

> Carsten Dominik <carsten.dominik@gmail.com> writes:
>
>> Hi Dan,
>>
>> OK, I have now applied the patch.  If you don't mind, could
>> you please double-check the commit?
>
> [Re.: commit 4b6988bf36cb458c9d113ee4332e016990c1eb04 ]
>
> Thanks Carsten, I looked over the commit and it looked good. However
> after using it a bit over the last week I have discovered two bugs  
> and a
> code issue. These follow below, together with proposed patches. There
> are 3 separate patches. Just in case you want to accept all 3 of them,
> I've created a single patch against current git right at the bottom of
> the email.
>
> But first, there are a couple of aesthetic choices being made here  
> that
> others might have better ideas for than the ones I've come up with so
> far. Firstly, how should we name edit buffers? At the moment we're  
> using
> my proposal of (actually I just added spaces inside the square  
> brackets)
>
> (concat "*Org Src " org-buffer-name "[ " lang " ]*")
>
> e.g.
>
> "*Org Src testing.org[ python ]*"
>
> Secondly, how shall we name edit buffers for Fixed Width / ASCII  
> drawing
> regions? The patch below uses
>
> (concat "*Org Src " org-buffer-name "[ " "Fixed Width" " ]*")
>
> e.g.
>
> "*Org Src testing.org[ Fixed Width ]*"
>
> (Maybe "Literal" would be better? Neither seems perfect.)
>
> Now for the bugs & patches.
>
> * Bug 1: C-x s switches active buffer
>  1. open two edit buffers simultaneously and modify them
>  2. now issue C-x s in one of the edit buffers
>  3. when it comes to saving the second edit buffer, it will steal  
> focus.
>
> Would an appropriate solution be to wrap the body of org-edit-src-save
> in a save-window-excursion, as in this patch? I've tried it quickly
> and it seems to have the desired effect.
>
> --8<---------------cut here---------------start------------->8---
> diff --git a/lisp/org-src.el b/lisp/org-src.el
> index fbf395f..df09f39 100644
> --- a/lisp/org-src.el
> +++ b/lisp/org-src.el
> @@ -474,14 +474,15 @@ the language, a switch telling of the content  
> should be in a single line."
> (defun org-edit-src-save ()
>   "Save parent buffer with current state source-code buffer."
>   (interactive)
> -  (let ((p (point)) (m (mark)) msg)
> -    (org-edit-src-exit)
> -    (save-buffer)
> -    (setq msg (current-message))
> -    (org-edit-src-code)
> -    (push-mark m 'nomessage)
> -    (goto-char (min p (point-max)))
> -    (message (or msg ""))))
> +  (save-window-excursion
> +    (let ((p (point)) (m (mark)) msg)
> +      (org-edit-src-exit)
> +      (save-buffer)
> +      (setq msg (current-message))
> +      (org-edit-src-code)
> +      (push-mark m 'nomessage)
> +      (goto-char (min p (point-max)))
> +      (message (or msg "")))))
> --8<---------------cut here---------------end--------------->8---
>
>
> * Bug 2: org-edit-src-find-buffer fails to find buffer
>  - Use C-c ' to switch to an edit buffer and make a modification
>  - Return to the org buffer without killing edit buffer
>  - Try C-c ' on the same code block
>  - You are now in a new edit buffer instead of returning to the  
> original one!
>
>  This is because it had not been updated to reflect the new edit  
> buffer
>  naming scheme. I propose that we ensure consistency by creating a new
>  function org-src-construct-edit-buffer-name which takes the parent
>  buffer name and the language and generates the edit buffer name, as  
> in
>  this patch.
>
> --8<---------------cut here---------------start------------->8---
> diff --git a/lisp/org-src.el b/lisp/org-src.el
> index df09f39..b0932c1 100644
> --- a/lisp/org-src.el
> +++ b/lisp/org-src.el
> @@ -185,7 +185,8 @@ the edited version."
>                (org-delete-overlay org-edit-src-overlay)))
>          (kill-buffer buffer))
>        (setq buffer (generate-new-buffer
> -                     (concat "*Org Src " (file-name-nondirectory  
> buffer-file-name) "[" lang "]*")))
> +                     (org-src-construct-edit-buffer-name
> +                      (file-name-nondirectory buffer-file-name)  
> lang)))
>        (setq ovl (org-make-overlay beg end))
>        (org-overlay-put ovl 'face 'secondary-selection)
>        (org-overlay-put ovl 'edit-buffer buffer)
> @@ -231,13 +232,17 @@ the edited version."
>     (if buf (switch-to-buffer buf)
>       (error "Something is wrong here"))))
>
> +(defun org-src-construct-edit-buffer-name (org-buffer-name lang)
> +  "Construct the buffer name for a source editing buffer"
> +  (concat "*Org Src " org-buffer-name "[ " lang " ]*"))
> +
> (defun org-edit-src-find-buffer (beg end)
>   "Find a source editing buffer that is already editing the region  
> BEG to END."
>   (catch 'exit
>     (mapc
>      (lambda (b)
>        (with-current-buffer b
> -        (if (and (string-match "\\`*Org Edit " (buffer-name))
> +        (if (and (string-match "\\`*Org Src " (buffer-name))
>                  (local-variable-p 'org-edit-src-beg-marker (current- 
> buffer))
>                  (local-variable-p 'org-edit-src-end-marker (current- 
> buffer))
>                  (equal beg org-edit-src-beg-marker)
> @@ -289,7 +294,9 @@ the fragment in the Org-mode buffer."
>            (if (boundp 'org-edit-src-overlay)
>                (org-delete-overlay org-edit-src-overlay)))
>          (kill-buffer buffer))
> -       (setq buffer (generate-new-buffer "*Org Edit Src Example*"))
> +       (setq buffer (generate-new-buffer
> +                     (org-src-construct-edit-buffer-name
> +                      (file-name-nondirectory buffer-file-name)  
> "Fixed Width")))
>        (setq ovl (org-make-overlay beg end))
>        (org-overlay-put ovl 'face 'secondary-selection)
>        (org-overlay-put ovl 'edit-buffer buffer)
> --8<---------------cut here---------------end--------------->8---
>
> * Code issue:
>  I can't work out why I was using (file-name-nondirectory
>  buffer-file-name) in order to construct the edit buffer name instead
>  of (buffer-name). Unless someone can think of a reason for this,
>  please apply the following patch.
>
> --8<---------------cut here---------------start------------->8---
> diff --git a/lisp/org-src.el b/lisp/org-src.el
> index b0932c1..5f21dfd 100644
> --- a/lisp/org-src.el
> +++ b/lisp/org-src.el
> @@ -185,8 +185,7 @@ the edited version."
>                (org-delete-overlay org-edit-src-overlay)))
>          (kill-buffer buffer))
>        (setq buffer (generate-new-buffer
> -                     (org-src-construct-edit-buffer-name
> -                      (file-name-nondirectory buffer-file-name)  
> lang)))
> +                     (org-src-construct-edit-buffer-name (buffer- 
> name) lang)))
>        (setq ovl (org-make-overlay beg end))
>        (org-overlay-put ovl 'face 'secondary-selection)
>        (org-overlay-put ovl 'edit-buffer buffer)
> @@ -295,8 +294,7 @@ the fragment in the Org-mode buffer."
>                (org-delete-overlay org-edit-src-overlay)))
>          (kill-buffer buffer))
>        (setq buffer (generate-new-buffer
> -                     (org-src-construct-edit-buffer-name
> -                      (file-name-nondirectory buffer-file-name)  
> "Fixed Width")))
> +                     (org-src-construct-edit-buffer-name (buffer- 
> name) "Fixed Width")))
>        (setq ovl (org-make-overlay beg end))
>        (org-overlay-put ovl 'face 'secondary-selection)
>        (org-overlay-put ovl 'edit-buffer buffer)
> --8<---------------cut here---------------end--------------->8---
>
>
> * Single unified patch
>
> --8<---------------cut here---------------start------------->8---
> diff --git a/lisp/org-src.el b/lisp/org-src.el
> index fbf395f..5f21dfd 100644
> --- a/lisp/org-src.el
> +++ b/lisp/org-src.el
> @@ -185,7 +185,7 @@ the edited version."
> 		(org-delete-overlay org-edit-src-overlay)))
> 	  (kill-buffer buffer))
> 	(setq buffer (generate-new-buffer
> -		      (concat "*Org Src " (file-name-nondirectory buffer-file- 
> name) "[" lang "]*")))
> +		      (org-src-construct-edit-buffer-name (buffer-name) lang)))
> 	(setq ovl (org-make-overlay beg end))
> 	(org-overlay-put ovl 'face 'secondary-selection)
> 	(org-overlay-put ovl 'edit-buffer buffer)
> @@ -231,13 +231,17 @@ the edited version."
>     (if buf (switch-to-buffer buf)
>       (error "Something is wrong here"))))
>
> +(defun org-src-construct-edit-buffer-name (org-buffer-name lang)
> +  "Construct the buffer name for a source editing buffer"
> +  (concat "*Org Src " org-buffer-name "[ " lang " ]*"))
> +
> (defun org-edit-src-find-buffer (beg end)
>   "Find a source editing buffer that is already editing the region  
> BEG to END."
>   (catch 'exit
>     (mapc
>      (lambda (b)
>        (with-current-buffer b
> -	 (if (and (string-match "\\`*Org Edit " (buffer-name))
> +	 (if (and (string-match "\\`*Org Src " (buffer-name))
> 		  (local-variable-p 'org-edit-src-beg-marker (current-buffer))
> 		  (local-variable-p 'org-edit-src-end-marker (current-buffer))
> 		  (equal beg org-edit-src-beg-marker)
> @@ -289,7 +293,8 @@ the fragment in the Org-mode buffer."
> 	    (if (boundp 'org-edit-src-overlay)
> 		(org-delete-overlay org-edit-src-overlay)))
> 	  (kill-buffer buffer))
> -	(setq buffer (generate-new-buffer "*Org Edit Src Example*"))
> +	(setq buffer (generate-new-buffer
> +		      (org-src-construct-edit-buffer-name (buffer-name) "Fixed  
> Width")))
> 	(setq ovl (org-make-overlay beg end))
> 	(org-overlay-put ovl 'face 'secondary-selection)
> 	(org-overlay-put ovl 'edit-buffer buffer)
> @@ -474,14 +479,15 @@ the language, a switch telling of the content  
> should be in a single line."
> (defun org-edit-src-save ()
>   "Save parent buffer with current state source-code buffer."
>   (interactive)
> -  (let ((p (point)) (m (mark)) msg)
> -    (org-edit-src-exit)
> -    (save-buffer)
> -    (setq msg (current-message))
> -    (org-edit-src-code)
> -    (push-mark m 'nomessage)
> -    (goto-char (min p (point-max)))
> -    (message (or msg ""))))
> +  (save-window-excursion
> +    (let ((p (point)) (m (mark)) msg)
> +      (org-edit-src-exit)
> +      (save-buffer)
> +      (setq msg (current-message))
> +      (org-edit-src-code)
> +      (push-mark m 'nomessage)
> +      (goto-char (min p (point-max)))
> +      (message (or msg "")))))
>
> (defun org-src-mode-configure-edit-buffer ()
>   (when org-edit-src-from-org-mode
> --8<---------------cut here---------------end--------------->8---
>
>
> Dan
>
>
>>
>> Thanks.
>>
>> - Carsten
>>
>> On Aug 28, 2009, at 4:36 AM, Dan Davison wrote:
>>
>>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>>>
>>>> Hi Dan,
>>>>
>>>> I am now finally looking at your patch.
>>>>
>>>> A few questions:
>>>>
>>>> On Aug 19, 2009, at 1:03 PM, Dan Davison wrote:
>>>>
>>>>> Dan Davison <davison@stats.ox.ac.uk> writes:
>>>>>
>>>>>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>>>>>>
>>>>>>> Hi Dan,
>>>>>>>
>>>>>>> thank you for studying and describing these issues, and for
>>>>>>> proposing
>>>>>>> a patch.
>>>>>
>>>>> I have noticed a bug in the patch I proposed: the configuration of
>>>>> the
>>>>> edit buffer for saving must be done only after C-c ', and not for
>>>>> example when entering org-src-mode during HTML export. Here's the
>>>>> revised patch (again, assuming org-edit-src-from-org-mode is a  
>>>>> valid
>>>>> test that C-c ' has just been done).
>>>>>
>>>>> Dan
>>>>>
>>>>> --8<---------------cut here---------------start------------->8---
>>>>> diff --git a/lisp/org-src.el b/lisp/org-src.el
>>>>> index 2a6c087..6ba58f5 100644
>>>>> --- a/lisp/org-src.el
>>>>> +++ b/lisp/org-src.el
>>>>> @@ -113,7 +113,6 @@ but which mess up the display of a snippet in
>>>>> Org exported files.")
>>>>>
>>>>> (defvar org-src-mode-map (make-sparse-keymap))
>>>>> (define-key org-src-mode-map "\C-c'" 'org-edit-src-exit)
>>>>> -(define-key org-src-mode-map "\C-x\C-s" 'org-edit-src-save)
>>>>> (defvar org-edit-src-force-single-line nil)
>>>>> (defvar org-edit-src-from-org-mode nil)
>>>>> (defvar org-edit-src-picture nil)
>>>>> @@ -168,7 +167,8 @@ the edited version."
>>>>> 	    (if (boundp 'org-edit-src-overlay)
>>>>> 		(org-delete-overlay org-edit-src-overlay)))
>>>>> 	  (kill-buffer buffer))
>>>>> -	(setq buffer (generate-new-buffer "*Org Edit Src Example*"))
>>>>> +	(setq buffer (generate-new-buffer
>>>>> +		      (concat "*Org Src " (file-name-nondirectory
>>>>> buffer-file-
>>>>> name) "[" lang "]*")))
>>>>> 	(setq ovl (org-make-overlay beg end))
>>>>> 	(org-overlay-put ovl 'face 'secondary-selection)
>>>>> 	(org-overlay-put ovl 'edit-buffer buffer)
>>>>> @@ -186,8 +186,7 @@ the edited version."
>>>>> 				'(display nil invisible nil intangible nil))
>>>>> 	(org-do-remove-indentation)
>>>>> 	(let ((org-inhibit-startup t))
>>>>> -	  (funcall lang-f)
>>>>> -	  (org-src-mode))
>>>>
>>>> You are moving the call to org-src-mode only so that you have
>>>> org-edit-
>>>> src-from-org defined in the hook, right?
>>>
>>> I also use the value of org-edit-src-beg-marker in the hook.
>>>
>>>>
>>>>> +	  (funcall lang-f))
>>>>> 	(set (make-local-variable 'org-edit-src-force-single-line)  
>>>>> single)
>>>>> 	(set (make-local-variable 'org-edit-src-from-org-mode) org-mode- 
>>>>> p)
>>>>> 	(when lfmt
>>>>> @@ -201,6 +200,7 @@ the edited version."
>>>>> 	(org-set-local 'org-edit-src-end-marker end)
>>>>> 	(org-set-local 'org-edit-src-overlay ovl)
>>>>> 	(org-set-local 'org-edit-src-nindent nindent)
>>>>> +	(org-src-mode)
>>>>> 	(and org-edit-src-persistent-message
>>>>> 	     (org-set-local 'header-line-format msg)))
>>>>>     (message "%s" msg)
>>>>> @@ -400,12 +400,13 @@ the language, a switch telling of the  
>>>>> content
>>>>> should be in a single line."
>>>>> (defun org-edit-src-exit ()
>>>>> "Exit special edit and protect problematic lines."
>>>>> (interactive)
>>>>> -  (unless (string-match "\\`*Org Edit " (buffer-name (current-
>>>>> buffer)))
>>>>> -    (error "This is not an sub-editing buffer, something is
>>>>> wrong..."))
>>>>> +  (unless org-edit-src-from-org-mode
>>>>> +    (error "This is not a sub-editing buffer, something is
>>>>> wrong..."))
>>>>> (let ((beg org-edit-src-beg-marker)
>>>>> 	(end org-edit-src-end-marker)
>>>>> 	(ovl org-edit-src-overlay)
>>>>> 	(buffer (current-buffer))
>>>>> +	(buffer-file-name nil)
>>>>
>>>> What is the above line for?
>>>
>>> I did this so that emacs allows us to kill a modified buffer without
>>> warning (Since, in the case of the src edit buffer, saving it  
>>> involves
>>> killing it). It seems that if emacs believes a buffer has no
>>> associated
>>> file it doesn't warn about modifications. However I think you've
>>> pointed
>>> to a less hackish way of doing this below. So instead of setting
>>> buffer-file-name to nil in this line that you are querying,  
>>> perhaps we
>>> should alter the end of org-edit-src-exit as follows:
>>>
>>>   ...
>>>   (setq code (buffer-string))
>>> +   (set-buffer-modified-p nil)
>>>   (switch-to-buffer (marker-buffer beg))
>>>   (kill-buffer buffer)
>>>   (goto-char beg)
>>>   (delete-region beg end)
>>>   (insert code)
>>>   ...
>>>
>>> ? This would be in addition to your suggestion of (set-buffer-
>>> modified-p
>>> nil) when entering org-src-mode.
>>>
>>>>
>>>>> 	(nindent org-edit-src-nindent)
>>>>> 	code line)
>>>>>   (untabify (point-min) (point-max))
>>>>> @@ -444,7 +445,6 @@ the language, a switch telling of the content
>>>>> should be in a single line."
>>>>>   (switch-to-buffer (marker-buffer beg))
>>>>>   (kill-buffer buffer)
>>>>>   (goto-char beg)
>>>>> -    (org-delete-overlay ovl)
>>>>
>>>> Why are you removing this line?
>>>
>>> The overlay should have been deleted already by the kill-buffer- 
>>> hook.
>>>
>>>>
>>>>>   (delete-region beg end)
>>>>>   (insert code)
>>>>>   (goto-char beg)
>>>>> @@ -464,6 +464,19 @@ the language, a switch telling of the content
>>>>> should be in a single line."
>>>>>   (goto-char (min p (point-max)))
>>>>>   (message (or msg ""))))
>>>>>
>>>>> +(defun org-src-mode-configure-edit-buffer ()
>>>>> +  (when org-edit-src-from-org-mode
>>>>> +    (setq buffer-offer-save t)
>>>>> +    (setq buffer-file-name
>>>>> +	  (concat (buffer-file-name (marker-buffer org-edit-src-beg-
>>>>> marker))
>>>>> +		  "[" (buffer-name) "]"))
>>>>> +    (set (if (featurep 'xemacs) 'write-contents-hooks 'write-
>>>>> contents-functions)
>>>>> +	 '(org-edit-src-save))
>>>>> +    (org-add-hook 'kill-buffer-hook
>>>>> +		  '(lambda () (org-delete-overlay
>>>>> org-edit-src-overlay)) nil 'local)))
>>>>> +
>>>>> +(org-add-hook 'org-src-mode-hook 'org-src-mode-configure-edit-
>>>>> buffer)
>>>>> +
>>>>> (provide 'org-src)
>>>>>
>>>>> ;; arch-tag: 6a1fc84f-dec7-47be-a416-64be56bea5d8
>>>>> --8<---------------cut here---------------end--------------->8---
>>>>
>>>>
>>>> I believe an important addition to your patch would be to
>>>>
>>>>  (set-buffer-modified-p nil)t
>>>>
>>>> when entering org-src-mode.  Otherwise, if I exit Emacs and reply  
>>>> "y"
>>>> to all safe-this-buffer questions, then I still get a complaint  
>>>> about
>>>> a buffer with changes....
>>>
>>> Yes, I think I agree (see above).
>>>
>>> Dan
>>>
>>>>
>>>> Thanks!
>>>>
>>>>
>>>>
>>>>
>>>> - Carsten
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Emacs-orgmode mailing list
>>>> Remember: use `Reply All' to send replies to the list.
>>>> Emacs-orgmode@gnu.org
>>>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode
>>
>>
>>
>> _______________________________________________
>> Emacs-orgmode mailing list
>> Remember: use `Reply All' to send replies to the list.
>> Emacs-orgmode@gnu.org
>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode

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

end of thread, other threads:[~2009-09-06 11:41 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-02 15:02 Saving the *Org Edit Src Example* buffer Dan Davison
2009-06-02 15:49 ` Dan Davison
2009-06-02 17:12   ` Carsten Dominik
2009-06-09 14:51     ` Dan Davison
2009-06-09 18:20       ` Carsten Dominik
2009-06-02 17:04 ` Carsten Dominik
2009-08-11 16:44   ` PATCH: proposed improvements to org-src-mode Dan Davison
2009-08-12 12:22     ` Carsten Dominik
2009-08-12 14:58       ` Dan Davison
2009-08-19 11:03         ` Dan Davison
2009-08-24 12:17           ` Carsten Dominik
2009-08-28  2:36             ` Dan Davison
2009-08-28  7:59               ` Carsten Dominik
2009-09-05 17:33                 ` Dan Davison
2009-09-06 11:41                   ` Carsten Dominik

Code repositories for project(s) associated with this 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).