From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Davison Subject: PATCH: proposed improvements to org-src-mode Date: Tue, 11 Aug 2009 12:44:52 -0400 Message-ID: <873a7y9u63.fsf_-_@stats.ox.ac.uk> References: <873aain0f7.fsf@stats.ox.ac.uk> <8216B89D-C718-4604-AAC4-82BFCA967161@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MauTA-0001mK-At for emacs-orgmode@gnu.org; Tue, 11 Aug 2009 12:45:04 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MauT4-0001km-Fk for emacs-orgmode@gnu.org; Tue, 11 Aug 2009 12:45:02 -0400 Received: from [199.232.76.173] (port=46954 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MauT4-0001kj-9a for emacs-orgmode@gnu.org; Tue, 11 Aug 2009 12:44:58 -0400 Received: from markov.stats.ox.ac.uk ([163.1.210.1]:37402) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MauT3-0007mM-Lb for emacs-orgmode@gnu.org; Tue, 11 Aug 2009 12:44:58 -0400 In-Reply-To: <8216B89D-C718-4604-AAC4-82BFCA967161@gmail.com> (Carsten Dominik's message of "Tue, 2 Jun 2009 19:04:56 +0200") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org 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 []*, where 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 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