Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bump clap2 to clap4-derive #306

Merged
merged 4 commits into from
Dec 10, 2022
Merged

bump clap2 to clap4-derive #306

merged 4 commits into from
Dec 10, 2022

Conversation

TD-Sky
Copy link
Contributor

@TD-Sky TD-Sky commented Dec 3, 2022

I only refactor the style of using clap, the semantic stay the same.

The only one broken change is, --fuzzy must have a value.

@cantino
Copy link
Owner

cantino commented Dec 8, 2022

Thanks for working on this @TD-Sky!

@TD-Sky
Copy link
Contributor Author

TD-Sky commented Dec 8, 2022

Sorry, I fixed the warning from clippy and retest it locally. It's OK now.

@cantino
Copy link
Owner

cantino commented Dec 10, 2022

Hey @TD-Sky, the following patch should make this compatible with master:

diff --cc src/settings.rs
index 472c5b4,23c8163..0000000
--- a/src/settings.rs
+++ b/src/settings.rs
diff --git a/src/cli.rs b/src/cli.rs
index 48d9591..53bfc76 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -42,9 +42,9 @@ pub enum SubCommand {
         #[arg(value_name = "EXIT_CODE", short, long)]
         exit: Option<i32>,
 
-        /// Also append new history to $HISTFILE/$MCFLY_HISTFILE (e.q., .bash_history)
-        #[arg(long)]
-        append_to_histfile: bool,
+        /// Also append command to the given file (e.q., .bash_history)
+        #[arg(value_name = "HISTFILE", short, long)]
+        append_to_histfile: Option<String>,
 
         /// The time that the command was run (default now)
         #[arg(value_name = "UNIX_EPOCH", short, long)]
@@ -82,7 +82,7 @@ pub enum SubCommand {
         delete_without_confirm: bool,
 
         /// Write results to file, including selection mode, new commandline, and any shell-specific requests
-        #[arg(short, long)]
+        #[arg(value_name = "PATH", short, long)]
         output_selection: Option<String>,
     },
diff --git a/src/settings.rs b/src/settings.rs
index 472c5b4..110aee2 100644
--- a/src/settings.rs
+++ b/src/settings.rs
@@ -74,7 +74,7 @@ pub struct Settings {
     pub when_run: Option<i64>,
     pub exit_code: Option<i32>,
     pub old_dir: Option<String>,
-    pub append_to_histfile: bool,
+    pub append_to_histfile: Option<String>,
     pub refresh_training_cache: bool,
     pub lightmode: bool,
     pub key_scheme: KeyScheme,
@@ -102,7 +102,7 @@ impl Default for Settings {
             exit_code: None,
             old_dir: None,
             refresh_training_cache: false,
-            append_to_histfile: false,
+            append_to_histfile: None,
             debug: false,
             fuzzy: 0,
             lightmode: false,

However, your PR isn't working for me. When I run mcfly init bash, for example, it doesn't print out anything. Have you tried installing mcfly after making your changes?

@TD-Sky
Copy link
Contributor Author

TD-Sky commented Dec 10, 2022

Hey @TD-Sky, the following patch should make this compatible with master:

diff --cc src/settings.rs
index 472c5b4,23c8163..0000000
--- a/src/settings.rs
+++ b/src/settings.rs
diff --git a/src/cli.rs b/src/cli.rs
index 48d9591..53bfc76 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -42,9 +42,9 @@ pub enum SubCommand {
         #[arg(value_name = "EXIT_CODE", short, long)]
         exit: Option<i32>,
 
-        /// Also append new history to $HISTFILE/$MCFLY_HISTFILE (e.q., .bash_history)
-        #[arg(long)]
-        append_to_histfile: bool,
+        /// Also append command to the given file (e.q., .bash_history)
+        #[arg(value_name = "HISTFILE", short, long)]
+        append_to_histfile: Option<String>,
 
         /// The time that the command was run (default now)
         #[arg(value_name = "UNIX_EPOCH", short, long)]
@@ -82,7 +82,7 @@ pub enum SubCommand {
         delete_without_confirm: bool,
 
         /// Write results to file, including selection mode, new commandline, and any shell-specific requests
-        #[arg(short, long)]
+        #[arg(value_name = "PATH", short, long)]
         output_selection: Option<String>,
     },
diff --git a/src/settings.rs b/src/settings.rs
index 472c5b4..110aee2 100644
--- a/src/settings.rs
+++ b/src/settings.rs
@@ -74,7 +74,7 @@ pub struct Settings {
     pub when_run: Option<i64>,
     pub exit_code: Option<i32>,
     pub old_dir: Option<String>,
-    pub append_to_histfile: bool,
+    pub append_to_histfile: Option<String>,
     pub refresh_training_cache: bool,
     pub lightmode: bool,
     pub key_scheme: KeyScheme,
@@ -102,7 +102,7 @@ impl Default for Settings {
             exit_code: None,
             old_dir: None,
             refresh_training_cache: false,
-            append_to_histfile: false,
+            append_to_histfile: None,
             debug: false,
             fuzzy: 0,
             lightmode: false,

However, your PR isn't working for me. When I run mcfly init bash, for example, it doesn't print out anything. Have you tried installing mcfly after making your changes?

I didn't realized my bad habit of not using the development version on my system in the past. I have did that and fixed the bug. There are nothing wrong happen.

@cantino cantino merged commit aceba98 into cantino:master Dec 10, 2022
@cantino
Copy link
Owner

cantino commented Dec 10, 2022

Thanks @TD-Sky, much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants