* orgtbl-mode and markdown
@ 2012-12-18 8:33 Vegard Vesterheim
2012-12-18 13:07 ` Carsten Dominik
0 siblings, 1 reply; 8+ messages in thread
From: Vegard Vesterheim @ 2012-12-18 8:33 UTC (permalink / raw)
To: emacs-orgmode
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: orgtbl-mode and markdown
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
0 siblings, 1 reply; 8+ messages in thread
From: Carsten Dominik @ 2012-12-18 13:07 UTC (permalink / raw)
To: Vegard Vesterheim; +Cc: emacs-orgmode
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? Looks to me that this is a bug in markdown-mode, which should be reported to its authors.
- Carsten
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: orgtbl-mode and markdown
2012-12-18 13:07 ` Carsten Dominik
@ 2012-12-18 13:39 ` Vegard Vesterheim
2012-12-18 14:50 ` Carsten Dominik
0 siblings, 1 reply; 8+ messages in thread
From: Vegard Vesterheim @ 2012-12-18 13:39 UTC (permalink / raw)
To: Carsten Dominik; +Cc: emacs-orgmode
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?
- Vegard V -
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: orgtbl-mode and markdown
2012-12-18 13:39 ` Vegard Vesterheim
@ 2012-12-18 14:50 ` Carsten Dominik
2012-12-18 17:56 ` Achim Gratz
0 siblings, 1 reply; 8+ messages in thread
From: Carsten Dominik @ 2012-12-18 14:50 UTC (permalink / raw)
To: Vegard Vesterheim; +Cc: emacs-orgmode
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: orgtbl-mode and markdown
2012-12-18 14:50 ` Carsten Dominik
@ 2012-12-18 17:56 ` Achim Gratz
2012-12-20 8:01 ` Carsten Dominik
0 siblings, 1 reply; 8+ messages in thread
From: Achim Gratz @ 2012-12-18 17:56 UTC (permalink / raw)
To: emacs-orgmode
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: orgtbl-mode and markdown
2012-12-18 17:56 ` Achim Gratz
@ 2012-12-20 8:01 ` Carsten Dominik
2012-12-21 10:53 ` Carsten Dominik
0 siblings, 1 reply; 8+ messages in thread
From: Carsten Dominik @ 2012-12-20 8:01 UTC (permalink / raw)
To: Achim Gratz; +Cc: emacs-orgmode
On 18.12.2012, at 18:56, Achim Gratz <Stromeko@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
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: orgtbl-mode and markdown
2012-12-20 8:01 ` Carsten Dominik
@ 2012-12-21 10:53 ` Carsten Dominik
2012-12-23 16:33 ` Bastien
0 siblings, 1 reply; 8+ messages in thread
From: Carsten Dominik @ 2012-12-21 10:53 UTC (permalink / raw)
To: Achim Gratz; +Cc: emacs-orgmode
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.dominik@gmail.com> wrote:
>
> On 18.12.2012, at 18:56, Achim Gratz <Stromeko@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: orgtbl-mode and markdown
2012-12-21 10:53 ` Carsten Dominik
@ 2012-12-23 16:33 ` Bastien
0 siblings, 0 replies; 8+ messages in thread
From: Bastien @ 2012-12-23 16:33 UTC (permalink / raw)
To: Carsten Dominik; +Cc: Achim Gratz, emacs-orgmode
Hi Vegard and Carsten,
Carsten Dominik <carsten.dominik@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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-12-23 16:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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
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).