Merging two copies of slightly divergent code

[Stefan Monnier]

I bumped into some funny duplication of code (one for Emacs and the other
for XEmacs) in flyspell.el and while trying to get rid of it, I noticed some
discrepencies between the two.  Could someone help me resolve
the differences?
See the questions I added in the patch.


        Stefan


--- orig/lisp/textmodes/flyspell.el
+++ mod/lisp/textmodes/flyspell.el
@@ -1959,61 +1946,30 @@
              ;; ispell error
              (error "Ispell: error in Ispell process"))
             ((string-match "GNU" (emacs-version))
-             ;; the word is incorrect, we have to propose a replacement
-             (setq replace (flyspell-emacs-popup event poss word))
-             (cond ((eq replace 'ignore)
-                    (goto-char save)
-                    nil)
-                   ((eq replace 'save)
-                    (goto-char save)
-                    (ispell-send-string (concat "*" word "\n"))
-                    (flyspell-unhighlight-at cursor-location)
-                    (setq ispell-pdict-modified-p '(t)))
-                   ((or (eq replace 'buffer) (eq replace 'session))
-                    (ispell-send-string (concat "@" word "\n"))
-                    (if (null ispell-pdict-modified-p)
-                        (setq ispell-pdict-modified-p
-                              (list ispell-pdict-modified-p)))
-                    (flyspell-unhighlight-at cursor-location)
-                    (goto-char save)
-                    (if (eq replace 'buffer)
-                        (ispell-add-per-file-word-list word)))
-                   (replace
-                    (flyspell-unhighlight-at cursor-location)
-                    (let ((new-word (if (atom replace)
-                                        replace
-                                      (car replace)))
-                          (cursor-location
-                           (+ (- (length word) (- end start))
-                              cursor-location)))
-                      (if (not (equal new-word (car poss)))
-                          (let ((old-max (point-max)))
-                            (delete-region start end)
-                            (funcall flyspell-insert-function new-word)
-                            (if flyspell-abbrev-p
-                                (flyspell-define-abbrev word new-word))
-                            (flyspell-ajust-cursor-point save
-                                                         cursor-location
-                                                         old-max)))))
-                   (t
-                    (goto-char save)
-                    nil)))
+             ;; The word is incorrect, we have to propose a replacement.
+              (flyspell-correct (flyspell-emacs-popup event poss word)
+                                poss word cursor-location start end save))
             ((eq flyspell-emacs 'xemacs)
              (flyspell-xemacs-popup
               event poss word cursor-location start end save)
-             (goto-char save)))
+              ))
            (ispell-pdict-save t))))))
 
 ;*---------------------------------------------------------------------*/
-;*    flyspell-xemacs-correct ...                                      */
+;*    flyspell-correct ...                                      */
 ;*---------------------------------------------------------------------*/
-(defun flyspell-xemacs-correct (replace poss word cursor-location start end 
save)
-  "The xemacs popup menu callback."
+(defun flyspell-correct (replace poss word cursor-location start end save)
+  "The popup menu callback."
+  ;; Originally, the XEmacs code didn't do the (goto-char save) here and did
+  ;; it instead right after calling the function.  Is this better?  --Stef
   (cond ((eq replace 'ignore)
+         (goto-char save)
         nil)
        ((eq replace 'save)
+         (goto-char save)
         (ispell-send-string (concat "*" word "\n"))
-        (ispell-send-string "#\n")
+         ;; This was only done on the XEmacs side.  Why?  --Stef
+        (if (featurep 'xemacs) (ispell-send-string "#\n"))
         (flyspell-unhighlight-at cursor-location)
         (setq ispell-pdict-modified-p '(t)))
        ((or (eq replace 'buffer) (eq replace 'session))
@@ -2022,9 +1978,13 @@
         (if (null ispell-pdict-modified-p)
             (setq ispell-pdict-modified-p
                   (list ispell-pdict-modified-p)))
+         (goto-char save)
         (if (eq replace 'buffer)
             (ispell-add-per-file-word-list word)))
        (replace
+         ;; Why only for Emacs?  It seems like it was just an oversight in
+         ;; the XEmacs side.  Could someone confirm?  --Stef
+         (if (featurep 'xemacs) nil (flyspell-unhighlight-at cursor-location))
         (let ((old-max (point-max))
               (new-word (if (atom replace)
                             replace
@@ -2038,7 +1998,12 @@
                 (funcall flyspell-insert-function new-word)
                 (if flyspell-abbrev-p
                     (flyspell-define-abbrev word new-word))))
-          (flyspell-ajust-cursor-point save cursor-location old-max)))))
+           ;; In the original Emacs code, this was only called in the body
+           ;; of the if.  Which one is right?  --Stef
+           (flyspell-ajust-cursor-point save cursor-location old-max)))
+        (t
+         (goto-char save)
+         nil)))
 
 ;*---------------------------------------------------------------------*/
 ;*    flyspell-ajust-cursor-point ...                                  */
@@ -2107,7 +2072,7 @@
         (cor-menu   (if (consp corrects)
                         (mapcar (lambda (correct)
                                   (vector correct
-                                          (list 'flyspell-xemacs-correct
+                                          (list 'flyspell-correct
                                                 correct
                                                 (list 'quote poss)
                                                 word
@@ -2122,7 +2087,7 @@
         (menu       (let ((save (if (consp affix)
                                     (vector
                                      (concat "Save affix: " (car affix))
-                                     (list 'flyspell-xemacs-correct
+                                     (list 'flyspell-correct
                                            ''save
                                            (list 'quote poss)
                                            word
@@ -2133,7 +2098,7 @@
                                      t)
                                   (vector
                                    "Save word"
-                                   (list 'flyspell-xemacs-correct
+                                   (list 'flyspell-correct
                                          ''save
                                          (list 'quote poss)
                                          word
@@ -2143,7 +2108,7 @@
                                          save)
                                    t)))
                           (session (vector "Accept (session)"
-                                           (list 'flyspell-xemacs-correct
+                                           (list 'flyspell-correct
                                                  ''session
                                                  (list 'quote poss)
                                                  word
@@ -2153,7 +2118,7 @@
                                                  save)
                                            t))
                           (buffer  (vector "Accept (buffer)"
-                                           (list 'flyspell-xemacs-correct
+                                           (list 'flyspell-correct
                                                  ''buffer
                                                  (list 'quote poss)
                                                  word