Merging two copies of slightly divergent code

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

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


_______________________________________________
Emacs-devel mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/emacs-devel
Reply | Threaded
Open this post in threaded view
|

Re: Merging two copies of slightly divergent code

Richard Stallman
It would be nice if Manuel Serrano responds, but if he does not,
would you please install whatever change you think is best?


_______________________________________________
Emacs-devel mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/emacs-devel