Bug 243975 - diff(1) should print the usage information when two different formatting options are used at the same time
Summary: diff(1) should print the usage information when two different formatting opti...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 12.1-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Kyle Evans
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2020-02-08 09:25 UTC by fehmi noyan isi
Modified: 2020-04-27 22:35 UTC (History)
4 users (show)

See Also:
kevans: mfc-stable12+
kevans: mfc-stable11+
kevans: mfc-stable10-


Attachments
patch for diff.c and tests/diff_test.sh (3.46 KB, patch)
2020-02-16 08:38 UTC, fehmi noyan isi
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 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 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 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 2020-04-27 22:35:38 UTC
Committed and MFC'd; thanks!
Comment 6 Kyle Evans freebsd_committer 2020-04-27 22:35:55 UTC
Committed and MFC'd; thanks!