From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Davison Subject: Re: Re: PATCH: proposed improvements to org-src-mode Date: Wed, 12 Aug 2009 10:58:45 -0400 Message-ID: <87iqgt5ba2.fsf@stats.ox.ac.uk> References: <873aain0f7.fsf@stats.ox.ac.uk> <8216B89D-C718-4604-AAC4-82BFCA967161@gmail.com> <873a7y9u63.fsf_-_@stats.ox.ac.uk> <9C5369DB-A034-4A0D-84FE-8872BB032BA6@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 1MbFI9-0004zB-0E for emacs-orgmode@gnu.org; Wed, 12 Aug 2009 10:59:05 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MbFI3-0004xS-9n for emacs-orgmode@gnu.org; Wed, 12 Aug 2009 10:59:03 -0400 Received: from [199.232.76.173] (port=40800 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MbFI3-0004xP-3u for emacs-orgmode@gnu.org; Wed, 12 Aug 2009 10:58:59 -0400 Received: from markov.stats.ox.ac.uk ([163.1.210.1]:40862) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MbFI2-0003ii-BH for emacs-orgmode@gnu.org; Wed, 12 Aug 2009 10:58:58 -0400 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 Carsten Dominik 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 []*, 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 > > > > _______________________________________________ > 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