emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Orgmode] Re: PATCH: proposed improvements to org-src-mode


From: Carsten Dominik
Subject: [Orgmode] Re: PATCH: proposed improvements to org-src-mode
Date: Wed, 12 Aug 2009 14:22:28 +0200

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 <address@hidden> 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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]