bug#1948: confusion and bug in dabbrev.el

[Peter Tury <tury.peter@gmail.com>]

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