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

From: Dan Davison <davison@stats.ox.ac.uk>
To: Carsten Dominik <carsten.dominik@gmail.com>
Cc: emacs org-mode mailing list <emacs-orgmode@gnu.org>
Subject: Re: Re: PATCH: proposed improvements to org-src-mode
Date: Fri, 28 Aug 2009 03:36:45 +0100	[thread overview]
Message-ID: <87bpm0vf42.fsf@stats.ox.ac.uk> (raw)
In-Reply-To: <3845FC6A-EF07-4CB0-BB36-6F2652CD4F26@gmail.com> (Carsten Dominik's message of "Mon, 24 Aug 2009 14:17:47 +0200")

Carsten Dominik <carsten.dominik@gmail.com> writes:

> Hi Dan,
>
> I am now finally looking at your patch.
>
> A few questions:
>
> On Aug 19, 2009, at 1:03 PM, Dan Davison wrote:
>
>> Dan Davison <davison@stats.ox.ac.uk> writes:
>>
>>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>>>
>>>> Hi Dan,
>>>>
>>>> thank you for studying and describing these issues, and for
>>>> proposing
>>>> a patch.
>>
>> I have noticed a bug in the patch I proposed: the configuration of the
>> edit buffer for saving must be done only after C-c ', and not for
>> example when entering org-src-mode during HTML export. Here's the
>> revised patch (again, assuming org-edit-src-from-org-mode is a valid
>> test that C-c ' has just been done).
>>
>> Dan
>>
>> --8<---------------cut here---------------start------------->8---
>> diff --git a/lisp/org-src.el b/lisp/org-src.el
>> index 2a6c087..6ba58f5 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))
>
> You are moving the call to org-src-mode only so that you have
> org-edit-
> src-from-org defined in the hook, right?

I also use the value of org-edit-src-beg-marker in the hook.

>
>> +	  (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)
>
> What is the above line for?

I did this so that emacs allows us to kill a modified buffer without
warning (Since, in the case of the src edit buffer, saving it involves
killing it). It seems that if emacs believes a buffer has no associated
file it doesn't warn about modifications. However I think you've pointed
to a less hackish way of doing this below. So instead of setting
buffer-file-name to nil in this line that you are querying, perhaps we
should alter the end of org-edit-src-exit as follows:

    ...
    (setq code (buffer-string))
+   (set-buffer-modified-p nil)
    (switch-to-buffer (marker-buffer beg))
    (kill-buffer buffer)
    (goto-char beg)
    (delete-region beg end)
    (insert code)
    ...

? This would be in addition to your suggestion of (set-buffer-modified-p
nil) when entering org-src-mode.

>
>> 	(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)
>
> Why are you removing this line?

The overlay should have been deleted already by the kill-buffer-hook.

>
>>     (delete-region beg end)
>>     (insert code)
>>     (goto-char beg)
>> @@ -464,6 +464,19 @@ 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-edit-buffer ()
>> +  (when org-edit-src-from-org-mode
>> +    (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-edit-buffer)
>> +
>> (provide 'org-src)
>>
>> ;; arch-tag: 6a1fc84f-dec7-47be-a416-64be56bea5d8
>> --8<---------------cut here---------------end--------------->8---
>
>
> I believe an important addition to your patch would be to
>
>    (set-buffer-modified-p nil)t
>
> when entering org-src-mode.  Otherwise, if I exit Emacs and reply "y"
> to all safe-this-buffer questions, then I still get a complaint about
> a buffer with changes....

Yes, I think I agree (see above).

Dan

>
> 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