emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Carsten Dominik <carsten.dominik@gmail.com>
To: Vegard Vesterheim <vegard.vesterheim@uninett.no>
Cc: emacs-orgmode@gnu.org
Subject: Re: orgtbl-mode and markdown
Date: Tue, 18 Dec 2012 15:50:48 +0100	[thread overview]
Message-ID: <66FAEC7A-9437-4ACD-A61E-4841AB8668EF@gmail.com> (raw)
In-Reply-To: <1sbodrbhsl.fsf@voll.uninett.no>


On 18 dec. 2012, at 14:39, Vegard Vesterheim <vegard.vesterheim@uninett.no> wrote:

> On Tue, 18 Dec 2012 14:07:36 +0100 Carsten Dominik <carsten.dominik@gmail.com> wrote:
> 
>> On 18 dec. 2012, at 09:33, Vegard Vesterheim <vegard.vesterheim@uninett.no> wrote:
>> 
>>> I had problems editing tables (using the minor mode orgtbl-mode) in
>>> markdown files. 
>>> 
>>> To reproduce: 
>>> - visit an empty buffer in markdown mode
>>> - M-x orgtbl-mode
>>> - create a new table (C-c |)
>>> - try to edit a cell
>>> - observe that the edited text is misplaced at the end of the line
>>> 
>>> The following patch against 709bf92950fb3e9dd7425e01eb53eedad43c7262
>>> seems to fix the problem
>>> 
>>> diff --git a/lisp/org-table.el b/lisp/org-table.el
>>> index acad0bb..188a825 100644
>>> --- a/lisp/org-table.el
>>> +++ b/lisp/org-table.el
>>> @@ -4233,10 +4233,10 @@ overwritten, and the table is not marked as requiring realignment."
>>> 	    t)
>>> 	   (eq N 1)
>>> 	   (looking-at "[^|\n]*  +|"))
>>> -      (let (org-table-may-need-update)
>>> -	(goto-char (1- (match-end 0)))
>>> -	(backward-delete-char 1)
>>> -	(goto-char (match-beginning 0))
>>> +      (let ((org-table-may-need-update) (mb (match-beginning 0)) (me (match-end 0)))
>>> +	(goto-char (1- me))
>>> +	(delete-backward-char 1)
>>> +	(goto-char mb)
>>> 	(self-insert-command N))
>>>    (setq org-table-may-need-update t)
>>>    (let* (orgtbl-mode
>> 
>> 
>> This is really strange.  Does markdown mode define hooks which kick in
>> on delete-backward-char and which to bad stuff with the match data?
> 
> Dunno. I just observed that (match-end 0) returned different results
> immediately before and after (backward-delete-char 1). 
> 
>> Looks to me that this is a bug in markdown-mode, which should be
>> reported to its authors.
> 
> Possibly, I am not proficient in elisp, hence the naive patch. But isn't
> it good practise to copy the match positions immediately after matching
> anyways? 

Well this is very safe - but if you have to assume that every elisp command does change match data behind your back, you would have to create a lot of extra code.  I think that you should report this to the markdown people.  I do not think we should have to make the change in org-mode.

I just checked markdown.el.  Below you can find a more complete description of what you can report to the markdown people:

Regards

- Carsten

Bug report for markdown.el
=====================================================================================

Markdown defines an after-change-function `markdown-check-change-for-wiki-link'.  This function is called after each buffer change.  The function contains a call to `markdown-fontify-region-wiki-links'.  If such a function would be called through the general font-lock stuff, the match data would be protected.  However, here it is called directly from a change function.  So this function should protect match data explicitly:

[lisp] Sir? diff -u markdown.el{.orig,}
--- markdown.el.orig	2012-12-18 15:28:18.000000000 +0100
+++ markdown.el	2012-12-18 15:46:50.000000000 +0100
@@ -1861,16 +1861,17 @@
 If a wiki link is found check to see if the backing file exists
 and highlight accordingly."
   (goto-char from)
-  (while (re-search-forward markdown-regex-wiki-link to t)
-    (let ((highlight-beginning (match-beginning 0))
-	  (highlight-end (match-end 0))
-	  (file-name
-	   (markdown-convert-wiki-link-to-filename (match-string 1))))
-      (if (file-exists-p file-name)
+  (save-match-data
+    (while (re-search-forward markdown-regex-wiki-link to t)
+      (let ((highlight-beginning (match-beginning 0))
+	    (highlight-end (match-end 0))
+	    (file-name
+	     (markdown-convert-wiki-link-to-filename (match-string 1))))
+	(if (file-exists-p file-name)
+	    (markdown-highlight-wiki-link
+	     highlight-beginning highlight-end markdown-link-face)
 	  (markdown-highlight-wiki-link
-	   highlight-beginning highlight-end markdown-link-face)
-	(markdown-highlight-wiki-link
-	 highlight-beginning highlight-end markdown-missing-link-face)))))
+	   highlight-beginning highlight-end markdown-missing-link-face))))))
 
 (defun markdown-extend-changed-region (from to)
   "Extend region given by FROM and TO so that we can fontify all links.

  reply	other threads:[~2012-12-18 14:50 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-18  8:33 orgtbl-mode and markdown Vegard Vesterheim
2012-12-18 13:07 ` Carsten Dominik
2012-12-18 13:39   ` Vegard Vesterheim
2012-12-18 14:50     ` Carsten Dominik [this message]
2012-12-18 17:56       ` Achim Gratz
2012-12-20  8:01         ` Carsten Dominik
2012-12-21 10:53           ` Carsten Dominik
2012-12-23 16:33             ` Bastien

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=66FAEC7A-9437-4ACD-A61E-4841AB8668EF@gmail.com \
    --to=carsten.dominik@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=vegard.vesterheim@uninett.no \
    /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).