From: Carsten Dominik <carsten.dominik@gmail.com>
To: Dan Davison <davison@stats.ox.ac.uk>
Cc: emacs org-mode mailing list <emacs-orgmode@gnu.org>
Subject: Re: PATCH: proposed improvements to org-src-mode
Date: Wed, 12 Aug 2009 14:22:28 +0200 [thread overview]
Message-ID: <9C5369DB-A034-4A0D-84FE-8872BB032BA6@gmail.com> (raw)
In-Reply-To: <873a7y9u63.fsf_-_@stats.ox.ac.uk>
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
next prev parent reply other threads:[~2009-08-12 12:22 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9C5369DB-A034-4A0D-84FE-8872BB032BA6@gmail.com \
--to=carsten.dominik@gmail.com \
--cc=davison@stats.ox.ac.uk \
--cc=emacs-orgmode@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).