all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: martin rudalics <rudalics@gmx.at>
To: emacs-devel <emacs-devel@gnu.org>
Subject: ediff-diff(3)-options
Date: Thu, 25 Oct 2007 08:45:52 +0200	[thread overview]
Message-ID: <47203BA0.1080605@gmx.at> (raw)

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

I conjecture that `ediff-diff-options' and `ediff-diff3-options' are not
implemented correctly on Windows systems for the following reasons:


(1) Trying to customize these options gives something like

ediff-diff-options: Hide Value --binary
     State: CHANGED outside Customize; operating on it here may be unreliable.

before I even tried to do anything here.  FWIW

      (set symb (concat mandatory-option val))

in `ediff-reset-diff-options' should become

      (set-default symb (concat mandatory-option val))


(2) When customizing `ediff-diff-options', `ediff-reset-diff-options'
will, via `ediff-diff-mandatory-option', always prepend one "--binary"
to the actual value.  Eventually, this will prepend as many "--binary"
to the option's value as the option has been customized.


(3) "--binary" is not supported by diff3 (at least not by version 2.8.7
which is the one I'm using).

According to the diffutils manual:

    If diff3 thinks that any of the files it is comparing is binary (a
    non-text file), it normally reports an error, because such comparisons
    are usually not useful.

Hence I don't understand why this should be set at all.  Does anyone
have a diff3 supporting the "--binary" option?


(4) Although "--binary" is not supported on my system
`ediff-test-utility' still returns 0 in the diff3 case.  I suspect that
this test is rather useless on Windows systems in its current form.


In my opinion most measurements are much too drastic to deal with the
trivial problem that some Windows users don't set "--binary" for diff.
I'd therefore propose to simplify the code as in the attached patch.


[-- Attachment #2: ediff.patch --]
[-- Type: text/plain, Size: 3653 bytes --]

*** ediff-diff.el.~1.61.~	Tue Sep 25 12:45:50 2007
--- ediff-diff.el	Wed Oct 24 23:16:24 2007
***************
*** 62,107 ****
  
  ;; The following functions must precede all defcustom-defined variables.
  
- ;; The following functions needed for setting diff/diff3 options
- ;; test if diff supports the --binary option
- (defsubst ediff-test-utility (diff-util option &optional files)
-   (condition-case nil
-       (eq 0 (apply 'call-process
- 		   (append (list diff-util nil nil nil option) files)))
-     (error (format "Cannot execute program %S." diff-util)))
-   )
- 
- (defun ediff-diff-mandatory-option (diff-util)
-   (let ((file (if (boundp 'null-device) null-device "/dev/null")))
-     (cond  ((not (memq system-type '(ms-dos windows-nt windows-95)))
- 	    "")
- 	   ((and (string= diff-util ediff-diff-program)
- 		 (ediff-test-utility
- 		  ediff-diff-program "--binary" (list file file)))
- 	    "--binary ")
- 	   ((and (string= diff-util ediff-diff3-program)
- 		 (ediff-test-utility
- 		  ediff-diff3-program "--binary" (list file file file)))
- 	    "--binary ")
- 	   (t ""))))
- 
- 
- ;; must be before ediff-reset-diff-options to avoid compiler errors
  (fset 'ediff-set-actual-diff-options '(lambda () nil))
  
- ;; make sure that mandatory options are added even if the user changes
- ;; ediff-diff-options or ediff-diff3-options in the customization widget
- (defun ediff-reset-diff-options (symb val)
-   (let* ((diff-program
- 	  (if (eq symb 'ediff-diff-options)
- 	      ediff-diff-program
- 	    ediff-diff3-program))
- 	 (mandatory-option (ediff-diff-mandatory-option diff-program)))
-     (set symb (concat mandatory-option val))
-     (ediff-set-actual-diff-options)
-     ))
- 
- 
  (defcustom ediff-shell
    (cond ((eq system-type 'emx) "cmd") ; OS/2
  	((memq system-type '(ms-dos windows-nt windows-95))
--- 62,69 ----
***************
*** 130,136 ****
    :type '(repeat string)
    :group 'ediff-diff)
  
! (defcustom ediff-diff-options ""
    "*Options to pass to `ediff-diff-program'.
  If Unix diff is used as `ediff-diff-program',
  then a useful option is `-w', to ignore space.
--- 92,103 ----
    :type '(repeat string)
    :group 'ediff-diff)
  
! (defun ediff-set-diff-options (symbol value)
!   (set symbol value)
!   (ediff-set-actual-diff-options))
! 
! (defcustom ediff-diff-options
!   (if (memq system-type '(ms-dos windows-nt windows-95)) "--binary" "")
    "*Options to pass to `ediff-diff-program'.
  If Unix diff is used as `ediff-diff-program',
  then a useful option is `-w', to ignore space.
***************
*** 140,146 ****
  This variable is not for customizing the look of the differences produced by
  the command \\[ediff-show-diff-output]. Use the variable
  `ediff-custom-diff-options' for that."
!   :set 'ediff-reset-diff-options
    :type 'string
    :group 'ediff-diff)
  
--- 107,113 ----
  This variable is not for customizing the look of the differences produced by
  the command \\[ediff-show-diff-output]. Use the variable
  `ediff-custom-diff-options' for that."
!   :set 'ediff-set-diff-options
    :type 'string
    :group 'ediff-diff)
  
***************
*** 179,185 ****
    "Pattern to match lines produced by diff3 that describe differences.")
  (defcustom ediff-diff3-options ""
    "*Options to pass to `ediff-diff3-program'."
!   :set 'ediff-reset-diff-options
    :type 'string
    :group 'ediff-diff)
  
--- 146,152 ----
    "Pattern to match lines produced by diff3 that describe differences.")
  (defcustom ediff-diff3-options ""
    "*Options to pass to `ediff-diff3-program'."
!   :set 'ediff-set-diff-options
    :type 'string
    :group 'ediff-diff)
  


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

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

                 reply	other threads:[~2007-10-25  6:45 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47203BA0.1080605@gmx.at \
    --to=rudalics@gmx.at \
    --cc=emacs-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.