all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#1948: confusion and bug in dabbrev.el
@ 2009-01-18 19:37 Peter Tury
  2012-09-12 19:22 ` Fred
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Peter Tury @ 2009-01-18 19:37 UTC (permalink / raw)
  To: emacs-pretest-bug

Hi,

1)
in `dabbrev--substitute-expansion' (in dabbrev.el), `t' value of
variable `dabbrev-eliminate-newlines' causes elimination of spaces
(and tabs) as well. Thus its name and its functionality are not really
compatible. In the best case it should have a better name
(`dabbrev-eliminate-whitespace'??), but as a minimum, its
documentation should be improved and mention elimination of tabs and
spaces as well, I think.

Moreover, contrary to its documentation again, it doesn't "converts
them [=newlines] to spaces": instead it just removes them. So its
documentation should be fixed here as well in my opinion.

2)
However, the more serious problem is that it "forgets" such
eliminations when it searches for `old' expansion in case of repeated
calls of  `dabbrev-expand' (i.e. when the check "(eq last-command
this-command)" in this function results t) what results in weird text
in some cases. To check this:

* customize `dabbrev-abbrev-char-regexp' to be a period (.) Thus
dabbrev-expand will replace whole lines.

* type these two lines into a buffer (with double spaces between the words!):
one  tt
one  ww

* then type "on" in the next line and call `dabbrev-expand' two times.
Your buffer should look like this at the end:
one  tt
one  ww
one tt
But instead you will get this:
one  tt
one tt
one ww
What seems to be a complete mess.

As I see the key here is the search for `old' at the end of
`dabbrev--substitute-expansion' (`old' is the first parameter of this
function). If I replace the following original code:
"    (if old
	(save-excursion
	  (search-backward old))"
to be
"    (if old
	;; Convert whitespace to single spaces.
	(progn
	(if dabbrev-eliminate-newlines
	    (let ((pos
		   (if (equal abbrev " ") 0 (length abbrev))))
	      ;; If ABBREV is real, search after the end of it.
	      ;; If ABBREV is space and we are copying successive words,
	      ;; search starting at the front.
	      (while (string-match "[\n \t]+" old pos)
		(setq pos (1+ (match-beginning 0)))
		(setq old (replace-match " " nil nil old)))))
	(save-excursion
	  (search-backward old)))"
my test (above) works well. Here I simply copy-pasted (...) original
whitespace-elimination code from few lines above, so this is clearly
not a nice solution: it just shows a way of fixing the broken
functionality.

(I "reported" this problem in gnu.emacs.help with the subject "strange
dabbrev for whole lines and double spaces in cvs Emacs 23" on
Jan.8.2009)

Could you please fix these bugs in dabbrev.el.

Thanks,
P






^ permalink raw reply	[flat|nested] 18+ messages in thread
* bug#1948: confusion and bug in dabbrev.el
@ 2009-01-19 14:54 Chong Yidong
  0 siblings, 0 replies; 18+ messages in thread
From: Chong Yidong @ 2009-01-19 14:54 UTC (permalink / raw)
  To: Lars Lindberg; +Cc: 1948, Peter Tury

Hi Lars, could you take a look at this bug report for dabbrev?  Thanks.

Peter Tury <tury.peter@gmail.com> wrote:

> 1) in `dabbrev--substitute-expansion' (in dabbrev.el), `t' value of
> variable `dabbrev-eliminate-newlines' causes elimination of spaces
> (and tabs) as well. Thus its name and its functionality are not really
> compatible. In the best case it should have a better name
> (`dabbrev-eliminate-whitespace'??), but as a minimum, its
> documentation should be improved and mention elimination of tabs and
> spaces as well, I think.

> Moreover, contrary to its documentation again, it doesn't "converts
> them [=newlines] to spaces": instead it just removes them. So its
> documentation should be fixed here as well in my opinion.

> 2) However, the more serious problem is that it "forgets" such
> eliminations when it searches for `old' expansion in case of repeated
> calls of `dabbrev-expand' (i.e. when the check "(eq last-command
> this-command)" in this function results t) what results in weird text
> in some cases. To check this:

> * customize `dabbrev-abbrev-char-regexp' to be a period (.) Thus
> dabbrev-expand will replace whole lines.

> * type these two lines into a buffer (with double spaces between the
>   words!):
> one  tt
> one  ww

> * then type "on" in the next line and call `dabbrev-expand' two times.
> Your buffer should look like this at the end:
> one  tt
> one  ww
> one tt
> But instead you will get this:
> one  tt
> one tt
> one ww
> What seems to be a complete mess.

> As I see the key here is the search for `old' at the end of
> `dabbrev--substitute-expansion' (`old' is the first parameter of this
> function). If I replace the following original code:
> "    (if old
>      (save-excursion
>        (search-backward old))"
> to be
> "    (if old
>      ;; Convert whitespace to single spaces.
>      (progn
>      (if dabbrev-eliminate-newlines
>          (let ((pos
>                  (if (equal abbrev " ") 0 (length abbrev))))
>                        ;; If ABBREV is real, search after the end of it.
>                              ;; If ABBREV is space and we are copying
>                              successive words,
>                                    ;; search starting at the front.
>                                          (while (string-match "[\n \t]+"
>                                          old pos)
>                                              (setq pos (1+
>                                              (match-beginning 0)))
>                                                               (setq old
>                                                               (replace-match
>                                                               " " nil
>                                                               nil
>                                                               old)))))
>                                                               (save-excursion
>                                                                 (search-backward
>                                                                 old)))"
> my test (above) works well. Here I simply copy-pasted (...) original
> whitespace-elimination code from few lines above, so this is clearly
> not a nice solution: it just shows a way of fixing the broken
> functionality.

> (I "reported" this problem in gnu.emacs.help with the subject "strange
> dabbrev for whole lines and double spaces in cvs Emacs 23" on
> Jan.8.2009)






^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2016-03-02 17:25 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-18 19:37 bug#1948: confusion and bug in dabbrev.el Peter Tury
2012-09-12 19:22 ` Fred
2016-01-09 23:36 ` Alan J Third
2016-01-10 13:41   ` Alan J Third
2016-01-30  1:01     ` Alan Third
2016-01-30  1:01     ` Alan Third
2016-01-30  2:00       ` Drew Adams
2016-01-30 11:59         ` Alan Third
2016-01-30 12:25           ` Eli Zaretskii
2016-01-30 17:37           ` Drew Adams
2016-02-29  4:18       ` Lars Ingebrigtsen
2016-03-01  3:16         ` Glenn Morris
2016-03-01  3:27           ` Lars Ingebrigtsen
2016-03-01  3:16         ` Glenn Morris
2016-02-29  4:18       ` Lars Ingebrigtsen
2016-03-01 18:19 ` bug#1948: [PATCH] Always save the actual expansion (bug#1948) Alan Third
2016-03-02 17:25   ` Lars Ingebrigtsen
  -- strict thread matches above, loose matches on Subject: below --
2009-01-19 14:54 bug#1948: confusion and bug in dabbrev.el Chong Yidong

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.