* bug#18824: split diff-check-labels off diff-no-select
@ 2014-10-25 9:17 Ivan Shmakov
2014-10-26 2:41 ` Stefan Monnier
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Ivan Shmakov @ 2014-10-25 9:17 UTC (permalink / raw)
To: 18824
[-- Attachment #1: Type: text/plain, Size: 394 bytes --]
Package: emacs
Severity: wishlist
Tags: patch
Please provide a separate diff-check-labels function to allow
for the code calling diff.el facilities to supply its own
--label= arguments to diff in place of diff.el-generated ones.
Possible patch, as well as the example usage, are MIMEd.
--
FSF associate member #7257 http://boycottsystemd.org/ … 3013 B6A0 230E 334A
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/diff, Size: 1265 bytes --]
--- a/lisp/vc/diff.el
+++ b/lisp/vc/diff.el
@@ -121,6 +121,15 @@ Possible values are:
nil -- no, it does not
check -- try to probe whether it does")
+(defun diff-check-labels (&optional force)
+ (if (not (or force (eq 'check diff-use-labels)))
+ diff-use-labels
+ (setq diff-use-labels
+ (with-temp-buffer
+ (when (ignore-errors
+ (call-process diff-command nil t nil "--help"))
+ (if (search-backward "--label" nil t) t))))))
+
(defun diff-no-select (old new &optional switches no-async buf)
;; Noninteractive helper for creating and reverting diff buffers
(unless (bufferp new) (setq new (expand-file-name new)))
@@ -128,11 +137,7 @@ diff-no-select (old new &optional switches no-async buf)
(or switches (setq switches diff-switches)) ; If not specified, use default.
(unless (listp switches) (setq switches (list switches)))
(or buf (setq buf (get-buffer-create "*Diff*")))
- (when (eq 'check diff-use-labels)
- (setq diff-use-labels
- (with-temp-buffer
- (when (ignore-errors (call-process diff-command nil t nil "--help"))
- (if (search-backward "--label" nil t) t)))))
+ (diff-check-labels)
(let* ((old-alt (diff-file-local-copy old))
(new-alt (diff-file-local-copy new))
(command
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Type: text/emacs-lisp, Size: 682 bytes --]
(let ((old "old") (new "new")
(target (get-buffer-create "*My Diff*"))
(labels (and (diff-check-labels)
(mapcar 'shell-quote-argument
(list "--label"
"*old revision*"
"--label"
"*new revision*")))))
(if labels
(let ((diff-use-labels nil)
(sw (nconc labels
(if (listp diff-switches)
diff-switches
(list diff-switches)))))
(diff-no-select old new switches nil target))
(diff-no-select old new nil nil target)))
^ permalink raw reply [flat|nested] 14+ messages in thread* bug#18824: split diff-check-labels off diff-no-select
2014-10-25 9:17 bug#18824: split diff-check-labels off diff-no-select Ivan Shmakov
@ 2014-10-26 2:41 ` Stefan Monnier
2014-10-27 10:25 ` bug#18850: smerge-mode: use diff-check-labels Ivan Shmakov
2016-02-23 12:05 ` bug#18824: split diff-check-labels off diff-no-select Lars Ingebrigtsen
2019-06-25 21:18 ` Lars Ingebrigtsen
2 siblings, 1 reply; 14+ messages in thread
From: Stefan Monnier @ 2014-10-26 2:41 UTC (permalink / raw)
To: Ivan Shmakov; +Cc: 18824
> Please provide a separate diff-check-labels function to allow
> for the code calling diff.el facilities to supply its own
> --label= arguments to diff in place of diff.el-generated ones.
Sounds fine. While you're at it, you might like to use this for
smerge-mode (which uses "-L" rather than "--label").
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread* bug#18850: smerge-mode: use diff-check-labels
2014-10-26 2:41 ` Stefan Monnier
@ 2014-10-27 10:25 ` Ivan Shmakov
2014-10-27 10:33 ` Ivan Shmakov
2014-11-21 10:57 ` bug#18850: (ping) Emacs bugs with patches? Ivan Shmakov
0 siblings, 2 replies; 14+ messages in thread
From: Ivan Shmakov @ 2014-10-27 10:25 UTC (permalink / raw)
To: 18850
[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]
Package: emacs
Severity: wishlist
Tags: patch
Control: block -1 by 18824
>>>>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Please provide a separate diff-check-labels function to allow for
>> the code calling diff.el facilities to supply its own --label=
>> arguments to diff in place of diff.el-generated ones.
> Sounds fine. While you're at it, you might like to use this for
> smerge-mode (which uses "-L" rather than "--label").
I assume you mean something along the lines of the (untested)
patch MIMEd.
Curiously, I don’t usually use the merge facility proper,
resolving merge conflicts afterwards. Instead, I save the diff
against the current revision, switch to a newer one, apply the
diff saved, and deal with the ‘.rej’ files, if any. Like:
$ (umask 0222 && git diff > +patch-$(date +%s).diff)
$ git reset origin/master \
&& patch -bVt -p1 -R < <(git diff)
…
$ patch -bVt -p1 < +patch-1414405170.diff
…
--
FSF associate member #7257 http://boycottsystemd.org/ … 3013 B6A0 230E 334A
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/diff, Size: 850 bytes --]
--- a/lisp/vc/smerge-mode.el
+++ b/lisp/vc/smerge-mode.el
@@ -44,6 +44,7 @@
;;; Code:
(eval-when-compile (require 'cl-lib))
+(require 'diff) ; for diff-check-labels
(require 'diff-mode) ;For diff-auto-refine-mode.
(require 'newcomment)
@@ -1162,10 +1163,13 @@ smerge-diff (n1 n2)
(erase-buffer)
(let ((status
(apply 'call-process diff-command nil t nil
- (append smerge-diff-switches
- (list "-L" (concat name1 "/" file)
- "-L" (concat name2 "/" file)
- file1 file2)))))
+ (nconc (and (diff-check-labels)
+ (list "--label"
+ (concat name1 "/" file)
+ "--label"
+ (concat name2 "/" file)))
+ smerge-diff-switches)
+ file1 file2)))))
(if (eq status 0) (insert "No differences found.\n"))))
(goto-char (point-min))
(diff-mode)
^ permalink raw reply [flat|nested] 14+ messages in thread* bug#18850: smerge-mode: use diff-check-labels
2014-10-27 10:25 ` bug#18850: smerge-mode: use diff-check-labels Ivan Shmakov
@ 2014-10-27 10:33 ` Ivan Shmakov
2014-10-27 13:32 ` Stefan Monnier
` (2 more replies)
2014-11-21 10:57 ` bug#18850: (ping) Emacs bugs with patches? Ivan Shmakov
1 sibling, 3 replies; 14+ messages in thread
From: Ivan Shmakov @ 2014-10-27 10:33 UTC (permalink / raw)
To: 18850, control
[-- Attachment #1: Type: text/plain, Size: 135 bytes --]
block 18850 by 18824
thanks
Another try.
--
FSF associate member #7257 http://boycottsystemd.org/ … 3013 B6A0 230E 334A
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/diff, Size: 840 bytes --]
--- a/lisp/vc/smerge-mode.el
+++ b/lisp/vc/smerge-mode.el
@@ -44,6 +44,7 @@
;;; Code:
(eval-when-compile (require 'cl-lib))
+(require 'diff) ; for diff-check-labels
(require 'diff-mode) ;For diff-auto-refine-mode.
(require 'newcomment)
@@ -1163,9 +1164,12 @@ repeating the command will highlight other two parts."
(let ((status
(apply 'call-process diff-command nil t nil
(append smerge-diff-switches
- (list "-L" (concat name1 "/" file)
- "-L" (concat name2 "/" file)
- file1 file2)))))
+ (and (diff-check-labels)
+ (list "--label"
+ (concat name1 "/" file)
+ "--label"
+ (concat name2 "/" file)))
+ (list file1 file2)))))
(if (eq status 0) (insert "No differences found.\n"))))
(goto-char (point-min))
(diff-mode)
^ permalink raw reply [flat|nested] 14+ messages in thread* bug#18850: smerge-mode: use diff-check-labels
2014-10-27 10:33 ` Ivan Shmakov
@ 2014-10-27 13:32 ` Stefan Monnier
2014-10-27 13:36 ` Ivan Shmakov
2016-02-23 12:03 ` Lars Ingebrigtsen
2019-06-25 21:22 ` Lars Ingebrigtsen
2 siblings, 1 reply; 14+ messages in thread
From: Stefan Monnier @ 2014-10-27 13:32 UTC (permalink / raw)
To: Ivan Shmakov; +Cc: control, 18850
> @@ -1163,9 +1164,12 @@ repeating the command will highlight other two parts."
> (let ((status
> (apply 'call-process diff-command nil t nil
> (append smerge-diff-switches
> - (list "-L" (concat name1 "/" file)
> - "-L" (concat name2 "/" file)
> - file1 file2)))))
> + (and (diff-check-labels)
> + (list "--label"
> + (concat name1 "/" file)
> + "--label"
> + (concat name2 "/" file)))
> + (list file1 file2)))))
> (if (eq status 0) (insert "No differences found.\n"))))
> (goto-char (point-min))
> (diff-mode)
That looks OK, yes. Tho you don't need to change -L to --label.
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread* bug#18850: smerge-mode: use diff-check-labels
2014-10-27 13:32 ` Stefan Monnier
@ 2014-10-27 13:36 ` Ivan Shmakov
2014-10-27 17:12 ` Stefan Monnier
0 siblings, 1 reply; 14+ messages in thread
From: Ivan Shmakov @ 2014-10-27 13:36 UTC (permalink / raw)
To: 18850
>>>>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
[…]
> That looks OK, yes. Tho you don't need to change -L to --label.
The point is that, strictly speaking, (diff-check-labels) only
checks for the --label option proper, not for its aliases.
--
FSF associate member #7257 http://boycottsystemd.org/ … 3013 B6A0 230E 334A
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#18850: smerge-mode: use diff-check-labels
2014-10-27 13:36 ` Ivan Shmakov
@ 2014-10-27 17:12 ` Stefan Monnier
2014-10-28 9:19 ` Ivan Shmakov
0 siblings, 1 reply; 14+ messages in thread
From: Stefan Monnier @ 2014-10-27 17:12 UTC (permalink / raw)
To: Ivan Shmakov; +Cc: 18850
>> That looks OK, yes. Tho you don't need to change -L to --label.
> The point is that, strictly speaking, (diff-check-labels) only
> checks for the --label option proper, not for its aliases.
Of course, part of the question, then is: are there diff commands that
handle one of the two but not the other. If there are, then we should
use the one that's more often supported, and if there aren't then it
doesn't matter.
So far, smerge-mode hasn't even bothered to check, so maybe the check
isn't that important any more (IIRC it was added for Solaris's diff,
tho I'd suspect that current derivatives of Solaris woujld accept
either -L or --label or both).
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread* bug#18850: smerge-mode: use diff-check-labels
2014-10-27 17:12 ` Stefan Monnier
@ 2014-10-28 9:19 ` Ivan Shmakov
2014-10-28 13:44 ` Stefan Monnier
0 siblings, 1 reply; 14+ messages in thread
From: Ivan Shmakov @ 2014-10-28 9:19 UTC (permalink / raw)
To: 18850
>>>>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> That looks OK, yes. Tho you don't need to change -L to --label.
>> The point is that, strictly speaking, (diff-check-labels) only
>> checks for the --label option proper, not for its aliases.
> Of course, part of the question, then is: are there diff commands
> that handle one of the two but not the other. If there are, then we
> should use the one that's more often supported, and if there aren't
> then it doesn't matter.
I have no reason to insist on doing it either way. However, it
still feels inconsistent to check for a specific option, and
then use its alias.
Then, however, there’s the following in diffutils/NEWS:
> User-visible changes in version 2.8:
[…]
> * The following diff options are still accepted, but are no longer documented.
> They may be withdrawn in future releases.
> -h (omit; it has no effect)
> -H (use --speed-large-files instead)
> -L (use --label instead)
(Version 2.8 was apparently released March, 2002.)
> So far, smerge-mode hasn't even bothered to check, so maybe the check
> isn't that important any more (IIRC it was added for Solaris's diff,
> tho I'd suspect that current derivatives of Solaris woujld accept
> either -L or --label or both).
Again, I have no reason to argue for retaining the check,
especially given that none of my systems seem to have diff(1)
versions other than the one from Diffutils, as of 3.0, or a
later version.
Also to note:
> User-visible changes in version 2.0:
[…]
> * diff options renamed:
> --label renamed from --file-label
Now, given that 2.7 – the oldest version available from
http://ftp.gnu.org/gnu/diffutils/ – dates back to 1994…
--
FSF associate member #7257 http://boycottsystemd.org/ … 3013 B6A0 230E 334A
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#18850: smerge-mode: use diff-check-labels
2014-10-27 10:33 ` Ivan Shmakov
2014-10-27 13:32 ` Stefan Monnier
@ 2016-02-23 12:03 ` Lars Ingebrigtsen
2019-06-25 21:22 ` Lars Ingebrigtsen
2 siblings, 0 replies; 14+ messages in thread
From: Lars Ingebrigtsen @ 2016-02-23 12:03 UTC (permalink / raw)
To: Ivan Shmakov; +Cc: 18850
Ivan Shmakov <ivan@siamics.net> writes:
> block 18850 by 18824
> thanks
>
> Another try.
The patch looks OK to me, and according to the discussion it sounds like
it was ready to be applied, but since that's more than a year ago, and
I'm not an smerge user, I wanted to ask whether the patch is still
something that should go in.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#18850: smerge-mode: use diff-check-labels
2014-10-27 10:33 ` Ivan Shmakov
2014-10-27 13:32 ` Stefan Monnier
2016-02-23 12:03 ` Lars Ingebrigtsen
@ 2019-06-25 21:22 ` Lars Ingebrigtsen
2 siblings, 0 replies; 14+ messages in thread
From: Lars Ingebrigtsen @ 2019-06-25 21:22 UTC (permalink / raw)
To: Ivan Shmakov; +Cc: 18850
Ivan Shmakov <ivan@siamics.net> writes:
> block 18850 by 18824
> thanks
>
> Another try.
OK; applied.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#18850: (ping) Emacs bugs with patches?
2014-10-27 10:25 ` bug#18850: smerge-mode: use diff-check-labels Ivan Shmakov
2014-10-27 10:33 ` Ivan Shmakov
@ 2014-11-21 10:57 ` Ivan Shmakov
1 sibling, 0 replies; 14+ messages in thread
From: Ivan Shmakov @ 2014-11-21 10:57 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 18824, 18850, 18246, 18175
Stefan, you’ve previously commented on several of the bug
reports I’ve suggested patches for. Could you please revisit
these patches and either commit or explicitly reject them?
(Any suggestion on how, if at all, do I improve them in the
latter case will be appreciated.)
The bug reports in question are as follows.
TIA.
http://debbugs.gnu.org/18175 Use (mapc 'switch-to-buffer …) in
files.el (was: mapcar.)
http://debbugs.gnu.org/18246 Use inhibit-point-motion-hooks
(in addition to inhibit-read-only) in enriched-encode.
http://debbugs.gnu.org/18824 New diff-check-labels function
(split off diff-no-select)
http://debbugs.gnu.org/18850 Use diff-check-labels and the --label
option to diff (was: -L – deprecated) in smerge-mode.el
--
FSF associate member #7257 np. По полям, за рекой — Иллет … B6A0 230E 334A
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#18824: split diff-check-labels off diff-no-select
2014-10-25 9:17 bug#18824: split diff-check-labels off diff-no-select Ivan Shmakov
2014-10-26 2:41 ` Stefan Monnier
@ 2016-02-23 12:05 ` Lars Ingebrigtsen
2019-06-25 21:18 ` Lars Ingebrigtsen
2 siblings, 0 replies; 14+ messages in thread
From: Lars Ingebrigtsen @ 2016-02-23 12:05 UTC (permalink / raw)
To: Ivan Shmakov; +Cc: 18824
Ivan Shmakov <ivan@siamics.net> writes:
> Package: emacs
> Severity: wishlist
> Tags: patch
>
> Please provide a separate diff-check-labels function to allow
> for the code calling diff.el facilities to supply its own
> --label= arguments to diff in place of diff.el-generated ones.
>
> Possible patch, as well as the example usage, are MIMEd.
I think this patch looks fine. Could you also provide a NEWS entry?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#18824: split diff-check-labels off diff-no-select
2014-10-25 9:17 bug#18824: split diff-check-labels off diff-no-select Ivan Shmakov
2014-10-26 2:41 ` Stefan Monnier
2016-02-23 12:05 ` bug#18824: split diff-check-labels off diff-no-select Lars Ingebrigtsen
@ 2019-06-25 21:18 ` Lars Ingebrigtsen
2 siblings, 0 replies; 14+ messages in thread
From: Lars Ingebrigtsen @ 2019-06-25 21:18 UTC (permalink / raw)
To: Ivan Shmakov; +Cc: 18824
Ivan Shmakov <ivan@siamics.net> writes:
> Please provide a separate diff-check-labels function to allow
> for the code calling diff.el facilities to supply its own
> --label= arguments to diff in place of diff.el-generated ones.
>
> Possible patch, as well as the example usage, are MIMEd.
I've now applied the patch along with the smerge one.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-06-25 21:22 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-25 9:17 bug#18824: split diff-check-labels off diff-no-select Ivan Shmakov
2014-10-26 2:41 ` Stefan Monnier
2014-10-27 10:25 ` bug#18850: smerge-mode: use diff-check-labels Ivan Shmakov
2014-10-27 10:33 ` Ivan Shmakov
2014-10-27 13:32 ` Stefan Monnier
2014-10-27 13:36 ` Ivan Shmakov
2014-10-27 17:12 ` Stefan Monnier
2014-10-28 9:19 ` Ivan Shmakov
2014-10-28 13:44 ` Stefan Monnier
2016-02-23 12:03 ` Lars Ingebrigtsen
2019-06-25 21:22 ` Lars Ingebrigtsen
2014-11-21 10:57 ` bug#18850: (ping) Emacs bugs with patches? Ivan Shmakov
2016-02-23 12:05 ` bug#18824: split diff-check-labels off diff-no-select Lars Ingebrigtsen
2019-06-25 21:18 ` Lars Ingebrigtsen
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).