From d6758445109ae0f52fc3dd09ccb88a95263217cb Mon Sep 17 00:00:00 2001
From: Andrew Gallant <jamslam@gmail.com>
Date: Fri, 7 Jul 2023 10:51:51 -0400
Subject: [PATCH] core: don't let context flags override eachother

This matches the behavior of GNU grep which does not ignore
before-context and after-context completely if the context flag is also
provided.

Note that this change wasn't done just to match GNU grep. In this case,
GNU grep has the more sensible behavior.

Fixes #2288, Closes #2451
---
 CHANGELOG.md        |  8 ++++++++
 crates/core/app.rs  | 17 ++++++-----------
 crates/core/args.rs |  6 +++---
 tests/feature.rs    | 17 +++++++++++++++++
 4 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d7d16a0d2..594c1f8ca 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,12 @@ TBD
 ===
 Unreleased changes. Release notes have not yet been written.
 
+**BREAKING CHANGES**
+
+* `rg -C1 -A2` used to be equivalent to `rg -A2`, but now it is equivalent to
+  `rg -B1 -A2`. That is, `-A` and `-B` no longer completely override `-C`.
+  Instead, they only partially override `-C`.
+
 Feature enhancements:
 
 * Added or improved file type filtering for Fuchsia, GraphQL
@@ -12,6 +18,8 @@ Bug fixes:
   Fix bug when using `-w` with a regex that can match the empty string.
 * [BUG #1911](https://github.com/BurntSushi/ripgrep/issues/1911):
   Disable mmap searching in all non-64-bit environments.
+* [BUG #2288](https://github.com/BurntSushi/ripgrep/issues/2288):
+  `-A` and `-B` now only each partially override `-C`.
 * [BUG #2236](https://github.com/BurntSushi/ripgrep/issues/2236):
   Fix gitignore parsing bug where a trailing `\/` resulted in an error.
 * [BUG #2480](https://github.com/BurntSushi/ripgrep/issues/2480):
diff --git a/crates/core/app.rs b/crates/core/app.rs
index 1fac5328b..fce7a2b44 100644
--- a/crates/core/app.rs
+++ b/crates/core/app.rs
@@ -698,7 +698,7 @@ fn flag_after_context(args: &mut Vec<RGArg>) {
         "\
 Show NUM lines after each match.
 
-This overrides the --context and --passthru flags.
+This overrides the --passthru flag and partially overrides --context.
 "
     );
     let arg = RGArg::flag("after-context", "NUM")
@@ -706,8 +706,7 @@ This overrides the --context and --passthru flags.
         .help(SHORT)
         .long_help(LONG)
         .number()
-        .overrides("passthru")
-        .overrides("context");
+        .overrides("passthru");
     args.push(arg);
 }
 
@@ -768,7 +767,7 @@ fn flag_before_context(args: &mut Vec<RGArg>) {
         "\
 Show NUM lines before each match.
 
-This overrides the --context and --passthru flags.
+This overrides the --passthru flag and partially overrides --context.
 "
     );
     let arg = RGArg::flag("before-context", "NUM")
@@ -776,8 +775,7 @@ This overrides the --context and --passthru flags.
         .help(SHORT)
         .long_help(LONG)
         .number()
-        .overrides("passthru")
-        .overrides("context");
+        .overrides("passthru");
     args.push(arg);
 }
 
@@ -1009,8 +1007,7 @@ fn flag_context(args: &mut Vec<RGArg>) {
 Show NUM lines before and after each match. This is equivalent to providing
 both the -B/--before-context and -A/--after-context flags with the same value.
 
-This overrides both the -B/--before-context and -A/--after-context flags,
-in addition to the --passthru flag.
+This overrides the --passthru flag.
 "
     );
     let arg = RGArg::flag("context", "NUM")
@@ -1018,9 +1015,7 @@ in addition to the --passthru flag.
         .help(SHORT)
         .long_help(LONG)
         .number()
-        .overrides("passthru")
-        .overrides("before-context")
-        .overrides("after-context");
+        .overrides("passthru");
     args.push(arg);
 }
 
diff --git a/crates/core/args.rs b/crates/core/args.rs
index 5ad038ed7..883bf0fb0 100644
--- a/crates/core/args.rs
+++ b/crates/core/args.rs
@@ -992,10 +992,10 @@ impl ArgMatches {
     /// If there was a problem parsing the values from the user as an integer,
     /// then an error is returned.
     fn contexts(&self) -> Result<(usize, usize)> {
-        let after = self.usize_of("after-context")?.unwrap_or(0);
-        let before = self.usize_of("before-context")?.unwrap_or(0);
         let both = self.usize_of("context")?.unwrap_or(0);
-        Ok(if both > 0 { (both, both) } else { (before, after) })
+        let after = self.usize_of("after-context")?.unwrap_or(both);
+        let before = self.usize_of("before-context")?.unwrap_or(both);
+        Ok((before, after))
     }
 
     /// Returns the unescaped context separator in UTF-8 bytes.
diff --git a/tests/feature.rs b/tests/feature.rs
index 36cbad73c..8283a1bba 100644
--- a/tests/feature.rs
+++ b/tests/feature.rs
@@ -921,6 +921,23 @@ rgtest!(f1842_field_match_separator, |dir: Dir, _: TestCommand| {
     eqnice!(expected, dir.command().args(&args).stdout());
 });
 
+// See: https://github.com/BurntSushi/ripgrep/issues/2288
+rgtest!(f2288_context_partial_override, |dir: Dir, mut cmd: TestCommand| {
+    dir.create("test", "1\n2\n3\n4\n5\n6\n7\n8\n9\n");
+    cmd.args(&["-C1", "-A2", "5", "test"]);
+    eqnice!("4\n5\n6\n7\n", cmd.stdout());
+});
+
+// See: https://github.com/BurntSushi/ripgrep/issues/2288
+rgtest!(
+    f2288_context_partial_override_rev,
+    |dir: Dir, mut cmd: TestCommand| {
+        dir.create("test", "1\n2\n3\n4\n5\n6\n7\n8\n9\n");
+        cmd.args(&["-A2", "-C1", "5", "test"]);
+        eqnice!("4\n5\n6\n7\n", cmd.stdout());
+    }
+);
+
 rgtest!(no_context_sep, |dir: Dir, mut cmd: TestCommand| {
     dir.create("test", "foo\nctx\nbar\nctx\nfoo\nctx");
     cmd.args(&["-A1", "--no-context-separator", "foo", "test"]);