Re: Re: PATCH: proposed improvements to org-src-mode

[Dan Davison <davison@stats.ox.ac.uk>]

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