Re: [O] orgtbl-mode and markdown
Hi Vegard and Carsten, Carsten Dominik carsten.domi...@gmail.com writes: OK, I withdraw my opposition to fixing this issue in org - by my criticism toward the way markdown.el defines this hook remains. I fixed this by saving match data for `org-delete-backward-char' and `org-delete-char'. I also sent a patch for Emacs -- let's see. Thanks! -- Bastien
Re: [O] orgtbl-mode and markdown
OK, I withdraw my opposition to fixing this issue in org - by my criticism toward the way markdown.el defines this hook remains. - Carsten On 20 dec. 2012, at 09:01, Carsten Dominik carsten.domi...@gmail.com wrote: On 18.12.2012, at 18:56, Achim Gratz strom...@nexgo.de wrote: Carsten Dominik writes: 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. Stefan Monnier advises the opposite (quote from http://article.gmane.org/gmane.emacs.bugs/68688 ): --8---cut here---start-8--- Okay. On second though: it wouldn't be very useful to use the match data set by these functions. Why not save the match-data here once so that no caller has to ever worry about it? Is there a general policy that this kinda thing shouldn't be done? Yes, the general policy is: all functions should be presumed to trash the match-data, except for a very few exceptions. --8---cut here---end---8--- I am sure that Stefan would consider backward-delete-charater such an exception, if you ask him directly. - Carsten Regards, Achim. -- +[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]+ SD adaptation for Waldorf rackAttack V1.04R1: http://Synth.Stromeko.net/Downloads.html#WaldorfSDada -- The graveyard is filled with indispensible people. -- Julia Sweeney's mother
Re: [O] orgtbl-mode and markdown
On 18.12.2012, at 18:56, Achim Gratz strom...@nexgo.de wrote: Carsten Dominik writes: 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. Stefan Monnier advises the opposite (quote from http://article.gmane.org/gmane.emacs.bugs/68688 ): --8---cut here---start-8--- Okay. On second though: it wouldn't be very useful to use the match data set by these functions. Why not save the match-data here once so that no caller has to ever worry about it? Is there a general policy that this kinda thing shouldn't be done? Yes, the general policy is: all functions should be presumed to trash the match-data, except for a very few exceptions. --8---cut here---end---8--- I am sure that Stefan would consider backward-delete-charater such an exception, if you ask him directly. - Carsten Regards, Achim. -- +[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]+ SD adaptation for Waldorf rackAttack V1.04R1: http://Synth.Stromeko.net/Downloads.html#WaldorfSDada
[O] orgtbl-mode and markdown
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
Re: [O] orgtbl-mode and markdown
On 18 dec. 2012, at 09:33, Vegard Vesterheim vegard.vesterh...@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? Looks to me that this is a bug in markdown-mode, which should be reported to its authors. - Carsten
Re: [O] orgtbl-mode and markdown
On Tue, 18 Dec 2012 14:07:36 +0100 Carsten Dominik carsten.domi...@gmail.com wrote: On 18 dec. 2012, at 09:33, Vegard Vesterheim vegard.vesterh...@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? - Vegard V -
Re: [O] orgtbl-mode and markdown
On 18 dec. 2012, at 14:39, Vegard Vesterheim vegard.vesterh...@uninett.no
wrote:
On Tue, 18 Dec 2012 14:07:36 +0100 Carsten Dominik
carsten.domi...@gmail.com wrote:
On 18 dec. 2012, at 09:33, Vegard Vesterheim vegard.vesterh...@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.orig2012-12-18 15:28:18.0 +0100
+++ markdown.el 2012-12-18 15:46:50.0 +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.
Re: [O] orgtbl-mode and markdown
Carsten Dominik writes: 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. Stefan Monnier advises the opposite (quote from http://article.gmane.org/gmane.emacs.bugs/68688 ): --8---cut here---start-8--- Okay. On second though: it wouldn't be very useful to use the match data set by these functions. Why not save the match-data here once so that no caller has to ever worry about it? Is there a general policy that this kinda thing shouldn't be done? Yes, the general policy is: all functions should be presumed to trash the match-data, except for a very few exceptions. --8---cut here---end---8--- Regards, Achim. -- +[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]+ SD adaptation for Waldorf rackAttack V1.04R1: http://Synth.Stromeko.net/Downloads.html#WaldorfSDada
