Bug 243975

Summary: diff(1) should print the usage information when two different formatting options are used at the same time
Product: Base System Reporter: fehmi noyan isi <fnoyanisi>
Component: binAssignee: Kyle Evans <kevans>
Status: Closed FIXED    
Severity: Affects Only Me CC: bugs, emaste, fnoyanisi, kevans
Priority: --- Keywords: patch
Version: 12.1-RELEASEFlags: kevans: mfc-stable12+
kevans: mfc-stable11+
kevans: mfc-stable10-
Hardware: Any   
OS: Any   
Attachments:
Description Flags
patch for diff.c and tests/diff_test.sh none

Description fehmi noyan isi 2020-02-08 09:25:15 UTC 
At the moment, when two different formatting options are used, diff(1) uses one of the output formatting styles (needs a bit of debugging to figure out which one is chosen - probably the last matching one) and ignores the other.
GNU diff displays an error message. Similarly, diff(1) should display a message and the usage information before exiting.

fnoyanisi@beastie:~ % cat A
same1
differ1


differ3
fnoyanisi@beastie:~ % cat B
same1  
differ2
differ4
fnoyanisi@beastie:~ % diff A B -u
--- A	2020-02-07 22:18:54.963051000 +0000
+++ B	2020-02-07 22:19:17.261555000 +0000
@@ -1,5 +1,3 @@
-same1
-differ1
-
-
-differ3
+same1  
+differ2
+differ4
fnoyanisi@beastie:~ % diff A B -e
1,5c
same1  
differ2
differ4
.
fnoyanisi@beastie:~ % diff A B -e -u
--- A	2020-02-07 22:18:54.963051000 +0000
+++ B	2020-02-07 22:19:17.261555000 +0000
@@ -1,5 +1,3 @@
-same1
-differ1
-
-
-differ3
+same1  
+differ2
+differ4
fnoyanisi@beastie:~ % uname -a
FreeBSD beastie 12.1-RELEASE-p2 FreeBSD 12.1-RELEASE-p2 GENERIC  amd64

I am working on a patch. Checking out the latest head source and building the world at the moment.
Comment 1 Kyle Evans freebsd_committer freebsd_triage 2020-02-13 21:01:28 UTC 
Take so I don't forget to close it after MFC.
Comment 2 fehmi noyan isi 2020-02-16 08:38:19 UTC 
Created attachment 211687 [details]
patch for diff.c and tests/diff_test.sh

Added a new test case to check conflicting formatting options (more can be added) and removed one of the atf tests from unified_body() (see the email [1])

The patch adds a check when a formatting option is encountered and when multiple options are used, it writes a message to stderr, displays the usage and exists.

This will align the behaviour with GNU diff 

[1] https://lists.freebsd.org/pipermail/freebsd-hackers/2020-February/055646.html
Comment 3 commit-hook freebsd_committer freebsd_triage 2020-04-20 16:15:29 UTC 
A commit references this bug:

Author: kevans
Date: Mon Apr 20 16:14:45 UTC 2020
New revision: 360125
URL: https://svnweb.freebsd.org/changeset/base/360125

Log:
  diff(1): reject conflicting formatting options

  This matches GNU diff(1) behavior and, more importantly, eliminates any
  source of confusion if multiple formatting options are specified.

  Note that the committed diff differs slightly from the submitted: I've
  modified it so that we initialize diff_format to something that isn't an
  accepted format option so that we can also reject --normal -c and -c
  --normal, which would've otherwise been accepted because the default was
  --normal. After option parsing we default it to D_NORMAL if it's still
  unset.

  PR:		243975
  Submitted by:	fehmi noyan isi
  MFC after:	1 week

Changes:
  head/usr.bin/diff/diff.c
  head/usr.bin/diff/diff.h
  head/usr.bin/diff/tests/diff_test.sh
Comment 4 commit-hook freebsd_committer freebsd_triage 2020-04-27 22:32:52 UTC 
A commit references this bug:

Author: kevans
Date: Mon Apr 27 22:32:16 UTC 2020
New revision: 360403
URL: https://svnweb.freebsd.org/changeset/base/360403

Log:
  MFC r360125: diff(1): reject conflicting formatting options

  This matches GNU diff(1) behavior and, more importantly, eliminates any
  source of confusion if multiple formatting options are specified.

  Note that the committed diff differs slightly from the submitted: I've
  modified it so that we initialize diff_format to something that isn't an
  accepted format option so that we can also reject --normal -c and -c
  --normal, which would've otherwise been accepted because the default was
  --normal. After option parsing we default it to D_NORMAL if it's still
  unset.

  PR:		243975

Changes:
_U  stable/12/
  stable/12/usr.bin/diff/diff.c
  stable/12/usr.bin/diff/diff.h
  stable/12/usr.bin/diff/tests/diff_test.sh
Comment 5 Kyle Evans freebsd_committer freebsd_triage 2020-04-27 22:35:38 UTC 
Committed and MFC'd; thanks!
Comment 6 Kyle Evans freebsd_committer freebsd_triage 2020-04-27 22:35:55 UTC 
Committed and MFC'd; thanks!