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
  2016-01-09 23:36 ` Alan J Third
  0 siblings, 2 replies; 16+ 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] 16+ messages in thread

* bug#1948: confusion and bug in dabbrev.el
@ 2009-01-19 14:54 Chong Yidong
  0 siblings, 0 replies; 16+ 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] 16+ messages in thread

* bug#1948: confusion and bug in dabbrev.el
  2009-01-18 19:37 Peter Tury
@ 2012-09-12 19:22 ` Fred
  2016-01-09 23:36 ` Alan J Third
  1 sibling, 0 replies; 16+ messages in thread
From: Fred @ 2012-09-12 19:22 UTC (permalink / raw)
  To: 1948, emacs-devel

Follow-up to http://debbugs.gnu.org/cgi/bugreport.cgi?bug=1948

I seem to be experiencing a bad version of this bug. In some instances,
dabbrev seems to feel like not only inserting text at point (which is
expected) but also modifying some text in other parts of the buffer. 
For
some reason, TCL code seems to trigger the bug quite easily. To see 
what
I experienced:

Save the following as blah.tcl:

====== beginning of blah.tcl
proc check { topology } {
     set network [ blahblah ]
     preconfig $topology $networks
     do_check  $topology $networks
}

# Apply the default pre-config steps for each device
proc preconfig { topology networks } {
     foreach node [ keylkeys topology ] {
	set config [ get_config -device $node -type pre ]
	apply_config $node $config
     }
}

# do the actual check
proc do_check {
====== end of blah.tcl

Start `emacs -Q', and either look at the `view-lossage' output included
below, or in plain English:

- Open blah.tcl
- Put the point after the opening brace of do_check;
- Insert `SPC top' and complete with `dabbrev-expand' (M-/) to get
   `topology';
- Insert `SPC' and try to complete (M-/) until you get `topology 
network'.

Expected:

After a round or two, dabbrev inserts networks, based on the prototype
of `preconfig'. At worse, it inserts rubbish and I have to give a 
longer
hint to dabbrev.

Actual result:

Some text after the last occurence of `topology' is replaced, point has
been moved, and procedure prototype has not been completed.

With Peter Tury's patch to `dabbrev--substitute-expansion', point does
not seem to start jumping around, replacing text in remote places
(though I don't have the correct replacement, but whatever)


In GNU Emacs 24.2.1 (x86_64-pc-linux-gnu, GTK+ Version 2.24.10)
  of 2012-09-09 on trouble, modified by Debian
Windowing system distributor `The X.Org Foundation', version 
11.0.11203902
Configured using:
  `configure '--build' 'x86_64-linux-gnu' '--build' 'x86_64-linux-gnu'
  '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib'
  '--localstatedir=/var/lib' '--infodir=/usr/share/info'
  '--mandir=/usr/share/man' '--with-pop=yes'
  
'--enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.2/site-lisp:/usr/share/emacs/site-lisp'
  '--with-crt-dir=/usr/lib/x86_64-linux-gnu' '--with-x=yes'
  '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars'
  'build_alias=x86_64-linux-gnu' 'CFLAGS=-g -O2 -fstack-protector
  --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wall'
  'CPPFLAGS=-D_FORTIFY_SOURCE=2''

Important settings:
   value of $LC_ALL: nil
   value of $LC_COLLATE: nil
   value of $LC_CTYPE: nil
   value of $LC_MESSAGES: nil
   value of $LC_MONETARY: nil
   value of $LC_NUMERIC: nil
   value of $LC_TIME: nil
   value of $LANG: en_GB.UTF-8
   value of $XMODIFIERS: nil
   locale-coding-system: utf-8-unix
   default enable-multibyte-characters: t

Major mode: Tcl

Minor modes in effect:
   tooltip-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   line-number-mode: t
   transient-mark-mode: t

Recent input:
C-x C-f / t m p / b l a h . t c l <return> C-s d o
_ c C-s C-e SPC t o p M-/ SPC M-/ M-/ C-/ C-/ M-x r
e p o <tab> r t - e m <tab> <return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Mark saved where search started
Undo! [2 times]
Making completion list...

Load-path shadows:
/usr/share/emacs/24.2/site-lisp/flim/hex-util hides 
/usr/share/emacs/24.2/lisp/hex-util
/usr/share/emacs/24.2/site-lisp/flim/md4 hides 
/usr/share/emacs/24.2/lisp/md4
/usr/share/emacs/24.2/site-lisp/dictionaries-common/flyspell hides 
/usr/share/emacs/24.2/lisp/textmodes/flyspell
/usr/share/emacs/24.2/site-lisp/dictionaries-common/ispell hides 
/usr/share/emacs/24.2/lisp/textmodes/ispell
/usr/share/emacs/24.2/site-lisp/flim/sasl-ntlm hides 
/usr/share/emacs/24.2/lisp/net/sasl-ntlm
/usr/share/emacs/24.2/site-lisp/flim/sasl-digest hides 
/usr/share/emacs/24.2/lisp/net/sasl-digest
/usr/share/emacs/24.2/site-lisp/flim/hmac-def hides 
/usr/share/emacs/24.2/lisp/net/hmac-def
/usr/share/emacs/24.2/site-lisp/flim/sasl hides 
/usr/share/emacs/24.2/lisp/net/sasl
/usr/share/emacs/24.2/site-lisp/flim/hmac-md5 hides 
/usr/share/emacs/24.2/lisp/net/hmac-md5
/usr/share/emacs/24.2/site-lisp/flim/ntlm hides 
/usr/share/emacs/24.2/lisp/net/ntlm
/usr/share/emacs/24.2/site-lisp/flim/sasl-cram hides 
/usr/share/emacs/24.2/lisp/net/sasl-cram

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 
mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils help-mode view dabbrev misearch multi-isearch tcl
easymenu comint regexp-opt ansi-color ring time-date tooltip ediff-hook
vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar dnd fontset image
fringe lisp-mode register page menu-bar rfn-eshadow timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame 
cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese hebrew greek romanian slovak czech european ethiopic
indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple
abbrev minibuffer loaddefs button faces cus-face files text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process dbusbind
dynamic-setting system-font-setting font-render-setting move-toolbar 
gtk
x-toolkit x multi-tty emacs)

-- 
Fred -- http://tar-jx.bz





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

* bug#1948: confusion and bug in dabbrev.el
  2009-01-18 19:37 Peter Tury
  2012-09-12 19:22 ` Fred
@ 2016-01-09 23:36 ` Alan J Third
  2016-01-10 13:41   ` Alan J Third
  1 sibling, 1 reply; 16+ messages in thread
From: Alan J Third @ 2016-01-09 23:36 UTC (permalink / raw)
  To: Peter Tury; +Cc: 1948

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

> * 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

I'm still getting this weird behaviour in 25.1, but the above doesn't
work exactly for me.

Set dabbrev-eliminate-newlines to "t".

Type:

one  tt
one  ww

Then in the next line I have to type:

one 
   ^- space

and M-/ (dabbrev-expand) completes it to:

one ww

Then if I hit M-/ again, point jumps up to the line above and replaces
"ww". Subsequent M-/'s continue to fiddle with the same line.
-- 
Alan Third





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

* bug#1948: confusion and bug in dabbrev.el
  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
  0 siblings, 2 replies; 16+ messages in thread
From: Alan J Third @ 2016-01-10 13:41 UTC (permalink / raw)
  To: 1948; +Cc: Peter Tury

Alan J Third <alan@idiocy.org> writes:

> Type:
>
> one  tt
> one  ww
>
> Then in the next line I have to type:
>
> one 
>    ^- space
>
> and M-/ (dabbrev-expand) completes it to:
>
> one ww
>
> Then if I hit M-/ again, point jumps up to the line above and replaces
> "ww". Subsequent M-/'s continue to fiddle with the same line.

I got this slightly wrong. The full set of commands is:

Type:

one  tt
one  ww
on M-/ SPC M-/ M-/

I've written a test, but I'm not sure what to do with it:

(ert-deftest dabbrev-expand-test ()
  "Test for bug#1948.
When DABBREV-ELIMINATE-NEWLINES is non-nil (the default),
repeated calls to DABBREV-EXPAND can result in the source of
first expansion being replaced rather than the destination."
  (with-temp-buffer
   (insert "ab  x\na\nab  y")
   (goto-char 8)
   (save-window-excursion
     (set-window-buffer nil (current-buffer))
     ;; M-/ SPC M-/ M-/
     (execute-kbd-macro "\257 \257\257"))
   (should (string= (buffer-string) "ab  x\nab y\nab  y"))))

I haven't signed the copyright assignment stuff.
-- 
Alan Third





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

* bug#1948: confusion and bug in dabbrev.el
  2016-01-10 13:41   ` Alan J Third
  2016-01-30  1:01     ` Alan Third
@ 2016-01-30  1:01     ` Alan Third
  1 sibling, 0 replies; 16+ messages in thread
From: Alan Third @ 2016-01-30  1:01 UTC (permalink / raw)
  To: 1948, emacs-devel; +Cc: Peter Tury

[-- Attachment #1: Type: text/plain, Size: 907 bytes --]

Peter Tury said:
> 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.

I think I've got a fix for this.

The problem is that dabbrev--substitute-expansion alters EXPANSION
before using it in the substitution, but dabbrev-expand has no idea that
it's done this, so it assumes the unaltered EXPANSION is still valid and
saves it.

The patch makes dabbrev--substitute-expansion return EXPANSION, and
dabbrev-expand saves that version instead.

I considered making a new function to remove the whitespace and using it
in both dabbrev-expand and dabbrev--substitute-expansion, but returning
the value actually used strikes me as safer, in case anything else is
done to it.


[-- Attachment #2: save actual expansion --]
[-- Type: text/plain, Size: 1470 bytes --]

From 9ef22a911d58f772265bc1fde2608ea135a4dea2 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 30 Jan 2016 00:38:32 +0000
Subject: [PATCH] Always save the actual expansion

lisp/dabbrev.el (dabbrev--substitute-expansion): Return EXPANSION after
any processing.
lisp/dabbrev.el (dabbrev-expand): Set EXPANSION to the return value of
DABBREV--SUBSTITUTE-EXPANSION.
---
 lisp/dabbrev.el | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lisp/dabbrev.el b/lisp/dabbrev.el
index 3557041..d9f36b1 100644
--- a/lisp/dabbrev.el
+++ b/lisp/dabbrev.el
@@ -546,8 +546,8 @@ dabbrev-expand
 		(copy-marker dabbrev--last-expansion-location)))
       ;; Success: stick it in and return.
       (setq buffer-undo-list (cons orig-point buffer-undo-list))
-      (dabbrev--substitute-expansion old abbrev expansion
-				     record-case-pattern)
+      (setq expansion (dabbrev--substitute-expansion old abbrev expansion
+                                                     record-case-pattern))
 
       ;; Save state for re-expand.
       (setq dabbrev--last-expansion expansion)
@@ -902,7 +902,9 @@ dabbrev--substitute-expansion
     ;; and (2) the replacement itself is all lower case.
     (dabbrev--safe-replace-match expansion
 				 (not use-case-replace)
-				 t)))
+				 t))
+  ;; Return the expansion actually used.
+  expansion)
 
 
 ;;;----------------------------------------------------------------
-- 
2.5.4 (Apple Git-61)


[-- Attachment #3: Type: text/plain, Size: 16 bytes --]


-- 
Alan Third

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

* Re: bug#1948: confusion and bug in dabbrev.el
  2016-01-10 13:41   ` Alan J Third
@ 2016-01-30  1:01     ` Alan Third
  2016-01-30  2:00       ` Drew Adams
                         ` (2 more replies)
  2016-01-30  1:01     ` Alan Third
  1 sibling, 3 replies; 16+ messages in thread
From: Alan Third @ 2016-01-30  1:01 UTC (permalink / raw)
  To: 1948, emacs-devel; +Cc: Peter Tury

[-- Attachment #1: Type: text/plain, Size: 907 bytes --]

Peter Tury said:
> 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.

I think I've got a fix for this.

The problem is that dabbrev--substitute-expansion alters EXPANSION
before using it in the substitution, but dabbrev-expand has no idea that
it's done this, so it assumes the unaltered EXPANSION is still valid and
saves it.

The patch makes dabbrev--substitute-expansion return EXPANSION, and
dabbrev-expand saves that version instead.

I considered making a new function to remove the whitespace and using it
in both dabbrev-expand and dabbrev--substitute-expansion, but returning
the value actually used strikes me as safer, in case anything else is
done to it.


[-- Attachment #2: save actual expansion --]
[-- Type: text/plain, Size: 1470 bytes --]

From 9ef22a911d58f772265bc1fde2608ea135a4dea2 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 30 Jan 2016 00:38:32 +0000
Subject: [PATCH] Always save the actual expansion

lisp/dabbrev.el (dabbrev--substitute-expansion): Return EXPANSION after
any processing.
lisp/dabbrev.el (dabbrev-expand): Set EXPANSION to the return value of
DABBREV--SUBSTITUTE-EXPANSION.
---
 lisp/dabbrev.el | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lisp/dabbrev.el b/lisp/dabbrev.el
index 3557041..d9f36b1 100644
--- a/lisp/dabbrev.el
+++ b/lisp/dabbrev.el
@@ -546,8 +546,8 @@ dabbrev-expand
 		(copy-marker dabbrev--last-expansion-location)))
       ;; Success: stick it in and return.
       (setq buffer-undo-list (cons orig-point buffer-undo-list))
-      (dabbrev--substitute-expansion old abbrev expansion
-				     record-case-pattern)
+      (setq expansion (dabbrev--substitute-expansion old abbrev expansion
+                                                     record-case-pattern))
 
       ;; Save state for re-expand.
       (setq dabbrev--last-expansion expansion)
@@ -902,7 +902,9 @@ dabbrev--substitute-expansion
     ;; and (2) the replacement itself is all lower case.
     (dabbrev--safe-replace-match expansion
 				 (not use-case-replace)
-				 t)))
+				 t))
+  ;; Return the expansion actually used.
+  expansion)
 
 
 ;;;----------------------------------------------------------------
-- 
2.5.4 (Apple Git-61)


[-- Attachment #3: Type: text/plain, Size: 16 bytes --]


-- 
Alan Third

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

* RE: bug#1948: confusion and bug in dabbrev.el
  2016-01-30  1:01     ` Alan Third
@ 2016-01-30  2:00       ` Drew Adams
  2016-01-30 11:59         ` Alan Third
  2016-02-29  4:18       ` Lars Ingebrigtsen
  2016-02-29  4:18       ` Lars Ingebrigtsen
  2 siblings, 1 reply; 16+ messages in thread
From: Drew Adams @ 2016-01-30  2:00 UTC (permalink / raw)
  To: Alan Third, emacs-devel

Please don't cross-post to the bugs list and emacs-devel.  Thx.



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

* Re: bug#1948: confusion and bug in dabbrev.el
  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
  0 siblings, 2 replies; 16+ messages in thread
From: Alan Third @ 2016-01-30 11:59 UTC (permalink / raw)
  To: Drew Adams; +Cc: emacs-devel

Drew Adams <drew.adams@oracle.com> writes:

> Please don't cross-post to the bugs list and emacs-devel.  Thx.

Apologies. Should I even be posting small patches for bug fixes in
emacs-devel, or should they just go to the bug's list?
-- 
Alan Third



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

* Re: bug#1948: confusion and bug in dabbrev.el
  2016-01-30 11:59         ` Alan Third
@ 2016-01-30 12:25           ` Eli Zaretskii
  2016-01-30 17:37           ` Drew Adams
  1 sibling, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2016-01-30 12:25 UTC (permalink / raw)
  To: Alan Third; +Cc: drew.adams, emacs-devel

> From: Alan Third <alan@idiocy.org>
> Date: Sat, 30 Jan 2016 11:59:08 +0000
> Cc: emacs-devel@gnu.org
> 
> Should I even be posting small patches for bug fixes in emacs-devel,
> or should they just go to the bug's list?

It is best to post only to the bug address.  All the relevant people
should read bug-gnu-emacs anyway, and if you need to CC someone
specifically, you can always do that while posting there.

Thanks.



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

* RE: bug#1948: confusion and bug in dabbrev.el
  2016-01-30 11:59         ` Alan Third
  2016-01-30 12:25           ` Eli Zaretskii
@ 2016-01-30 17:37           ` Drew Adams
  1 sibling, 0 replies; 16+ messages in thread
From: Drew Adams @ 2016-01-30 17:37 UTC (permalink / raw)
  To: Alan Third; +Cc: emacs-devel

> > Please don't cross-post to the bugs list and emacs-devel.  Thx.
> 
> Apologies. Should I even be posting small patches for bug fixes in
> emacs-devel, or should they just go to the bug's list?

Someone will correct me if I'm wrong, but I believe that either
is OK.  I'd suggest the bug list, unless you think that some
general discussion about it might be in order.



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

* bug#1948: confusion and bug in dabbrev.el
  2016-01-30  1:01     ` Alan Third
  2016-01-30  2:00       ` Drew Adams
  2016-02-29  4:18       ` Lars Ingebrigtsen
@ 2016-02-29  4:18       ` Lars Ingebrigtsen
  2 siblings, 0 replies; 16+ messages in thread
From: Lars Ingebrigtsen @ 2016-02-29  4:18 UTC (permalink / raw)
  To: Alan Third; +Cc: 1948, Peter Tury, emacs-devel

Alan Third <alan@idiocy.org> writes:

> I think I've got a fix for this.
>
> The problem is that dabbrev--substitute-expansion alters EXPANSION
> before using it in the substitution, but dabbrev-expand has no idea that
> it's done this, so it assumes the unaltered EXPANSION is still valid and
> saves it.
>
> The patch makes dabbrev--substitute-expansion return EXPANSION, and
> dabbrev-expand saves that version instead.
>
> I considered making a new function to remove the whitespace and using it
> in both dabbrev-expand and dabbrev--substitute-expansion, but returning
> the value actually used strikes me as safer, in case anything else is
> done to it.

If I understand the code correctly, this looks like a good fix to me.
Could you resubmit it with the test case you sent earlier as one patch?

Also I see that we have a copyright disclaimer on file for your work.
Is that sufficient to accept code into Emacs?  (I'm asking the other
Emacs hackers here...)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* Re: bug#1948: confusion and bug in dabbrev.el
  2016-01-30  1:01     ` Alan Third
  2016-01-30  2:00       ` Drew Adams
@ 2016-02-29  4:18       ` Lars Ingebrigtsen
  2016-03-01  3:16         ` Glenn Morris
  2016-03-01  3:16         ` Glenn Morris
  2016-02-29  4:18       ` Lars Ingebrigtsen
  2 siblings, 2 replies; 16+ messages in thread
From: Lars Ingebrigtsen @ 2016-02-29  4:18 UTC (permalink / raw)
  To: Alan Third; +Cc: 1948, Peter Tury, emacs-devel

Alan Third <alan@idiocy.org> writes:

> I think I've got a fix for this.
>
> The problem is that dabbrev--substitute-expansion alters EXPANSION
> before using it in the substitution, but dabbrev-expand has no idea that
> it's done this, so it assumes the unaltered EXPANSION is still valid and
> saves it.
>
> The patch makes dabbrev--substitute-expansion return EXPANSION, and
> dabbrev-expand saves that version instead.
>
> I considered making a new function to remove the whitespace and using it
> in both dabbrev-expand and dabbrev--substitute-expansion, but returning
> the value actually used strikes me as safer, in case anything else is
> done to it.

If I understand the code correctly, this looks like a good fix to me.
Could you resubmit it with the test case you sent earlier as one patch?

Also I see that we have a copyright disclaimer on file for your work.
Is that sufficient to accept code into Emacs?  (I'm asking the other
Emacs hackers here...)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* bug#1948: confusion and bug in dabbrev.el
  2016-02-29  4:18       ` Lars Ingebrigtsen
@ 2016-03-01  3:16         ` Glenn Morris
  2016-03-01  3:16         ` Glenn Morris
  1 sibling, 0 replies; 16+ messages in thread
From: Glenn Morris @ 2016-03-01  3:16 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 1948, Peter Tury, Alan Third, emacs-devel


(Insert standard "why is this on two mailing lists?" blurb here)

Lars Ingebrigtsen wrote:

> Also I see that we have a copyright disclaimer on file for your work.
> Is that sufficient to accept code into Emacs?  (I'm asking the other
> Emacs hackers here...)

A disclaimer is fine, though an assignment is preferred.
In this case, I see an assignment in the file.
Maybe you are looking at the employer disclaimer?

(I usually try grep "First.*Last" rather than just "First Last".)





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

* Re: bug#1948: confusion and bug in dabbrev.el
  2016-02-29  4:18       ` Lars Ingebrigtsen
  2016-03-01  3:16         ` Glenn Morris
@ 2016-03-01  3:16         ` Glenn Morris
  2016-03-01  3:27           ` Lars Ingebrigtsen
  1 sibling, 1 reply; 16+ messages in thread
From: Glenn Morris @ 2016-03-01  3:16 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 1948, Peter Tury, Alan Third, emacs-devel


(Insert standard "why is this on two mailing lists?" blurb here)

Lars Ingebrigtsen wrote:

> Also I see that we have a copyright disclaimer on file for your work.
> Is that sufficient to accept code into Emacs?  (I'm asking the other
> Emacs hackers here...)

A disclaimer is fine, though an assignment is preferred.
In this case, I see an assignment in the file.
Maybe you are looking at the employer disclaimer?

(I usually try grep "First.*Last" rather than just "First Last".)



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

* bug#1948: confusion and bug in dabbrev.el
  2016-03-01  3:16         ` Glenn Morris
@ 2016-03-01  3:27           ` Lars Ingebrigtsen
  0 siblings, 0 replies; 16+ messages in thread
From: Lars Ingebrigtsen @ 2016-03-01  3:27 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 1948, Peter Tury, Alan Third

Glenn Morris <rgm@gnu.org> writes:

> A disclaimer is fine, though an assignment is preferred.
> In this case, I see an assignment in the file.
> Maybe you are looking at the employer disclaimer?

Ah, yes.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

end of thread, other threads:[~2016-03-01  3:27 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-19 14:54 bug#1948: confusion and bug in dabbrev.el Chong Yidong
  -- strict thread matches above, loose matches on Subject: below --
2009-01-18 19:37 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  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:16         ` Glenn Morris
2016-03-01  3:27           ` Lars Ingebrigtsen
2016-02-29  4:18       ` Lars Ingebrigtsen
2016-01-30  1:01     ` Alan Third

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.