From 895c903b61f7745b6e3f2ca7c6c4b95d1c35df1b Mon Sep 17 00:00:00 2001
From: hk <alexhaka10@protonmail.com>
Date: Mon, 9 Nov 2020 15:29:00 +0100
Subject: [PATCH] refactor: adding get_global, making two functions private

preserving the observable behavior of the existing
public api, while handling global arguments separately in
get_arg_conflicts_with
---
 clap_generate/src/generators/shells/zsh.rs |  4 +-
 src/build/app/mod.rs                       | 70 ++++++++++++----------
 src/build/arg/mod.rs                       |  5 ++
 3 files changed, 46 insertions(+), 33 deletions(-)

diff --git a/clap_generate/src/generators/shells/zsh.rs b/clap_generate/src/generators/shells/zsh.rs
index 2d9e779e..010b73c3 100644
--- a/clap_generate/src/generators/shells/zsh.rs
+++ b/clap_generate/src/generators/shells/zsh.rs
@@ -490,9 +490,9 @@ fn arg_conflicts(app: &App, arg: &Arg, app_global: Option<&App>) -> String {
     }
 
     let mut res = vec![];
-    match (app_global, App::test_global(arg)) {
+    match (app_global, arg.get_global()) {
         (Some(x), true) => {
-            let conflicts = x.get_global_arg_conflicts_with(arg);
+            let conflicts = x.get_arg_conflicts_with(arg);
 
             if conflicts.is_empty() {
                 return String::new();
diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs
index 0fd2597a..e6ae738d 100644
--- a/src/build/app/mod.rs
+++ b/src/build/app/mod.rs
@@ -236,22 +236,20 @@ impl<'help> App<'help> {
         self.get_opts().filter(|a| a.get_help_heading().is_none())
     }
 
-    /// Test if the provided Argument is a global Argument
-    pub fn test_global(arg: &Arg) -> bool {
-        arg.global
-    }
-
-    /// Get a list of subcommands which contain the provided Argument
-    ///
-    /// This command will only include subcommands in its list for which the subcommands
-    /// parent also contains the Argument.
-    ///
-    /// **NOTE:** In this case only Sucommand_1 will be included
-    ///   Subcommand_1 (contains Arg)
-    ///     Subcommand_1.1 (doesn't contain Arg)
-    ///       Subcommand_1.1.1 (contains Arg)
-    ///
-    pub fn get_subcommands_containing(&self, arg: &Arg) -> Vec<&App<'help>> {
+    // Get a list of subcommands which contain the provided Argument
+    //
+    // This command will only include subcommands in its list for which the subcommands
+    // parent also contains the Argument.
+    //
+    // This search follows the propagation rules of global arguments.
+    // It is useful to finding subcommands, that have inherited a global argument.
+    //
+    // **NOTE:** In this case only Sucommand_1 will be included
+    //   Subcommand_1 (contains Arg)
+    //     Subcommand_1.1 (doesn't contain Arg)
+    //       Subcommand_1.1.1 (contains Arg)
+    //
+    fn get_subcommands_containing(&self, arg: &Arg) -> Vec<&App<'help>> {
         let mut vec = std::vec::Vec::new();
         for idx in 0..self.subcommands.len() {
             if self.subcommands[idx]
@@ -275,13 +273,16 @@ impl<'help> App<'help> {
         vec
     }
 
-    /// Get a unique list of all arguments of all commands and continuous subcommands the given argument conflicts with.
-    ///
-    /// ### Panics
-    ///
-    /// If the given arg contains a conflict with an argument that is unknown to
-    /// this `App`.
-    pub fn get_global_arg_conflicts_with(&self, arg: &Arg) -> Vec<&Arg<'help>> // FIXME: This could probably have been an iterator
+    // Get a unique list of all arguments of all commands and continuous subcommands the given argument conflicts with.
+    //
+    // This behavior follows the propagation rules of global arguments.
+    // It is useful for finding conflicts for arguments declared as global.
+    //
+    // ### Panics
+    //
+    // If the given arg contains a conflict with an argument that is unknown to
+    // this `App`.
+    fn get_global_arg_conflicts_with(&self, arg: &Arg) -> Vec<&Arg<'help>> // FIXME: This could probably have been an iterator
     {
         arg.blacklist
             .iter()
@@ -305,21 +306,28 @@ impl<'help> App<'help> {
 
     /// Get a list of all arguments the given argument conflicts with.
     ///
+    /// If the provided argument is declared as global, the conflicts will be determined
+    /// based on the propagation rules of global arguments.
+    ///
     /// ### Panics
     ///
     /// If the given arg contains a conflict with an argument that is unknown to
     /// this `App`.
     pub fn get_arg_conflicts_with(&self, arg: &Arg) -> Vec<&Arg<'help>> // FIXME: This could probably have been an iterator
     {
-        arg.blacklist
-            .iter()
-            .map(|id| {
-                self.args.args.iter().find(|arg| arg.id == *id).expect(
-                    "App::get_arg_conflicts_with: \
+        if arg.global {
+            self.get_global_arg_conflicts_with(arg)
+        } else {
+            arg.blacklist
+                .iter()
+                .map(|id| {
+                    self.args.args.iter().find(|arg| arg.id == *id).expect(
+                        "App::get_arg_conflicts_with: \
                     The passed arg conflicts with an arg unknown to the app",
-                )
-            })
-            .collect()
+                    )
+                })
+                .collect()
+        }
     }
 
     /// Returns `true` if the given [`AppSettings`] variant is currently set in
diff --git a/src/build/arg/mod.rs b/src/build/arg/mod.rs
index 3b8b8680..4fa3cd07 100644
--- a/src/build/arg/mod.rs
+++ b/src/build/arg/mod.rs
@@ -172,6 +172,11 @@ impl<'help> Arg<'help> {
     pub fn get_value_hint(&self) -> ValueHint {
         self.value_hint
     }
+
+    /// Get information on if this argument is global or not
+    pub fn get_global(&self) -> bool {
+        self.global
+    }
 }
 
 impl<'help> Arg<'help> {