slb: make trailing comment fixing opt-in and skip ignored paths

Cargo.toml

@@ -26,6 +26,7 @@ dunce = "1.0.5"
 encoding_rs = "0.8.41"
 futures = "0.3.34"
 git2 = { version = "0.21.0", features = ["ssh", "https"] }
+ignore = "0.4.33"
 indicatif = { version = "0.18.6", features = ["tokio", "futures", "rayon"] }
 itertools = "0.15.0"
 num_cpus = "1.17.0"

README.md

@@ -494,14 +494,19 @@ whenever the clause it introduces carries on past it,
 so the explanation or list starts on a line of its own.
 Text inside backticks, Markdown links, and inline formatting such as `**bold**`, `_italic_`,
 and `~~strikethrough~~` is never broken, and a span that was split by hand is joined back together.
-Semicolons are rewritten as separate sentences, and trailing comments are moved above the code.
+Semicolons are rewritten as separate sentences.
 An em dash becomes a period and a new sentence, or a colon where the text before it names what follows.
 A pair of dashes that encloses an aside becomes a pair of commas,
 and a dash that is kept never ends or starts a line.
 Aligned column blocks, such as the environment table of a usage comment, are left as they are.
 Slash-delimited regex literals are protected across supported source languages.
 When ambiguous slash syntax could hide a multiline string or comment, the remaining source is left unchanged.
+With `--trailing`, a comment sharing a line with code is moved onto its own line above it.
+That rule rewrites code lines and the result often reads worse than the original, so it is off by default.
+A trailing comment whose code line already has a comment above it is reported but not moved,
+since the two notes would be reflowed into one sentence.
 The line limit is read from project config files such as `.editorconfig`, `rustfmt.toml`, and `pyproject.toml`.
+Paths ignored by git are skipped, unless `--no-ignore` says otherwise.
 Files are checked in parallel, one worker per core unless `--jobs` says otherwise,
 and the report is printed in file order so a run is reproducible.
 The default mode reports violations and exits with code 1.
@@ -526,9 +531,11 @@ Options:
   -j, --join-sentences          Also pack consecutive short sentences up to the line limit
   -w, --width <N>               Maximum line length including indentation and comment marker (default: from project config or 120)
   -i, --ignore-project-config   Do not read the line length from project config files such as .editorconfig, rustfmt.toml, or pyproject.toml
-  -R, --rules <RULES>           Rules to enable (default: all) [possible values: too-long, mid-clause, semicolon, em-dash, trailing]
+  -R, --rules <RULES>           Rules to enable (default: every rule except trailing comments) [possible values: too-long, mid-clause, semicolon, em-dash, trailing]
+  -T, --trailing                Also move trailing comments to their own line above the code
   -e, --extensions <EXTENSION>  Only process files with these extensions
-  -x, --exclude <PATTERN>       Skip paths with a directory or file name equal to this text
+  -x, --exclude <PATTERN>       Skip paths with a directory or file name equal to this text, in addition to the default excludes
+  -n, --no-ignore               Do not skip paths ignored by git
   -t, --type <KIND>             Force the file kind, required with --stdin [possible values: rust, c, javascript, go, python, shell, toml, yaml, dockerfile, makefile, ruby, sql, lua, markdown]
   -s, --stdin                   Read text from stdin and write the formatted result to stdout
   -b, --word-break              Allow breaking at a plain word boundary when no clause boundary fits

cli-tools.toml

@@ -366,7 +366,10 @@ replace = [
 # Read the line length from project config files such as .editorconfig, rustfmt.toml, and pyproject.toml
 # use_project_config = true
 
-# Rules to enable
+# Rules to enable.
+# The "trailing" rule moves a comment sharing a line with code onto its own line above.
+# It rewrites code lines and the result often reads worse than the original,
+# so it is off unless listed here or asked for with --trailing.
 # rules = ["too-long", "mid-clause", "semicolon", "em-dash", "trailing"]
 
 # Also pack consecutive short sentences up to the line limit
@@ -378,9 +381,14 @@ replace = [
 # Only process files with these extensions (empty means all supported file types)
 # extensions = []
 
-# Skip paths with a directory or file name equal to any of these
+# Skip paths with a directory or file name equal to any of these.
+# Setting this replaces the built-in list, while --exclude on the command line adds to it.
 # exclude = ["target", "node_modules", "build", "dist", "cdk.out", "coverage", ".venv", "venv", "vendor"]
 
+# Skip paths ignored by git, using .gitignore, .ignore, the global ignore file, and .git/info/exclude.
+# Only applies inside a git repository, elsewhere the exclude list above is the only filter.
+# use_gitignore = true
+
 # Abbreviations that do not end a sentence (added to the built-in list)
 # abbreviations = []
 

src/bin/semantic_line_breaks/config.rs

@@ -52,6 +52,8 @@ pub struct SlbConfig {
     #[serde(default)]
     pub rules: Vec<String>,
     #[serde(default)]
+    pub use_gitignore: Option<bool>,
+    #[serde(default)]
     pub use_project_config: Option<bool>,
     #[serde(default)]
     pub verbose: bool,
@@ -85,6 +87,7 @@ pub struct Config {
     pub rules: RuleSet,
     pub stdin: bool,
     pub project_width: bool,
+    pub use_gitignore: bool,
     pub verbose: bool,
     pub width: Option<usize>,
 }
@@ -126,7 +129,9 @@ impl Config {
     ///
     /// Command line values win over the config file, which wins over the built-in defaults.
     /// Extension lists (abbreviations, directive prefixes, lowercase words) add to the defaults.
-    /// Clause starters and excludes replace the defaults when given.
+    /// Clause starters replace the defaults when given.
+    /// An exclude list in the config file replaces the defaults,
+    /// while command line excludes add to whichever list was resolved.
     ///
     /// # Errors
     /// Returns an error if the config file cannot be read or parsed or names an unknown rule.
@@ -139,9 +144,9 @@ impl Config {
     /// # Errors
     /// Returns an error if the user config names an unknown rule.
     pub fn from_args_and_user_config(args: &Args, user_config: SlbConfig) -> Result<Self> {
-        let rules = if args.rules.is_empty() {
+        let mut rules = if args.rules.is_empty() {
             if user_config.rules.is_empty() {
-                RuleSet::ALL
+                RuleSet::DEFAULT
             } else {
                 let kinds = user_config
                     .rules
@@ -156,16 +161,22 @@ impl Config {
         } else {
             RuleSet::from_kinds(&args.rules)
         };
+        if args.trailing {
+            rules.trailing_comment = true;
+        }
 
-        let exclude = if args.exclude.is_empty() {
-            if user_config.exclude.is_empty() {
-                cli_tools::strings_from(DEFAULT_EXCLUDES)
-            } else {
-                user_config.exclude
-            }
+        // The command line excludes narrow the walk further,
+        // so they add to the resolved list instead of replacing it.
+        let mut exclude = if user_config.exclude.is_empty() {
+            cli_tools::strings_from(DEFAULT_EXCLUDES)
         } else {
-            args.exclude.clone()
+            user_config.exclude
         };
+        for pattern in &args.exclude {
+            if !exclude.contains(pattern) {
+                exclude.push(pattern.clone());
+            }
+        }
 
         let extensions = if args.extensions.is_empty() {
             user_config.extensions
@@ -193,6 +204,7 @@ impl Config {
             rules,
             stdin: args.stdin,
             project_width: !args.ignore_project_config && user_config.use_project_config.unwrap_or(true),
+            use_gitignore: !args.no_ignore && user_config.use_gitignore.unwrap_or(true),
             verbose: args.verbose || user_config.verbose,
             width: args.width.or(user_config.width),
         })
@@ -308,7 +320,8 @@ mod test_config_from_args {
         assert!(config.rules.em_dash);
         assert!(!config.rules.trailing_comment);
         assert_eq!(config.width, Some(80));
-        assert_eq!(config.exclude, vec!["docs"]);
+        // The fixture config sets exclude = ["target"], and the command line value adds to it.
+        assert_eq!(config.exclude, vec!["target", "docs"]);
         assert!(config.project_width);
     }
 
@@ -319,7 +332,8 @@ mod test_config_from_args {
     fn fixture_config_values_are_merged_with_defaults() {
         let args = Args::try_parse_from(["slb", "--extensions", ".RS"]).expect("arguments should parse");
         let config = Config::from_args(&args).expect("config should build");
-        assert_eq!(config.rules, RuleSet::ALL);
+        // The fixture config lists every rule except the opt-in trailing comment rule.
+        assert_eq!(config.rules, RuleSet::DEFAULT);
         assert_eq!(config.exclude, vec!["target"]);
         assert_eq!(config.extensions, vec!["rs"]);
         assert_eq!(config.clause_starters, vec!["meanwhile"]);
@@ -358,7 +372,8 @@ mod test_config_merge {
     #[test]
     fn an_empty_user_config_gives_the_built_in_defaults() {
         let config = config(&["slb"], "").expect("config should build");
-        assert_eq!(config.rules, RuleSet::ALL);
+        assert_eq!(config.rules, RuleSet::DEFAULT);
+        assert!(!config.rules.trailing_comment);
         assert_eq!(config.exclude, cli_tools::strings_from(DEFAULT_EXCLUDES));
         assert!(config.extensions.is_empty());
         assert!(config.width.is_none());
@@ -398,7 +413,47 @@ mod test_config_merge {
         assert_eq!(config.exclude, vec!["generated"]);
 
         let overridden = config_with_exclude_option();
-        assert_eq!(overridden.exclude, vec!["docs"]);
+        assert_eq!(overridden.exclude, vec!["generated", "docs"]);
+    }
+
+    #[test]
+    fn a_command_line_exclude_adds_to_the_built_in_defaults() {
+        let config = config(&["slb", "--exclude", "docs"], "").expect("config should build");
+        assert!(config.exclude.contains(&"target".to_string()));
+        assert!(config.exclude.contains(&"node_modules".to_string()));
+        assert_eq!(config.exclude.last(), Some(&"docs".to_string()));
+    }
+
+    #[test]
+    fn a_command_line_exclude_already_in_the_list_is_not_repeated() {
+        let config = config(&["slb", "--exclude", "target"], "").expect("config should build");
+        assert_eq!(config.exclude, cli_tools::strings_from(DEFAULT_EXCLUDES));
+    }
+
+    #[test]
+    fn the_trailing_flag_adds_the_trailing_comment_rule() {
+        let default_rules = config(&["slb", "--trailing"], "").expect("config should build");
+        assert_eq!(default_rules.rules, RuleSet::ALL);
+
+        let with_rules = config(&["slb", "--rules", "semicolon", "--trailing"], "").expect("config should build");
+        assert!(with_rules.rules.semicolon);
+        assert!(with_rules.rules.trailing_comment);
+        assert!(!with_rules.rules.line_too_long);
+    }
+
+    #[test]
+    fn gitignore_is_used_unless_turned_off() {
+        assert!(config(&["slb"], "").expect("config should build").use_gitignore);
+        assert!(
+            !config(&["slb", "--no-ignore"], "")
+                .expect("config should build")
+                .use_gitignore
+        );
+        assert!(
+            !config(&["slb"], "[slb]\nuse_gitignore = false\n")
+                .expect("config should build")
+                .use_gitignore
+        );
     }
 
     /// Config with an exclude given on the command line and another one in the user config.
@@ -445,6 +500,6 @@ mod test_config_merge {
         assert_eq!(options.clause_starters, vec!["meanwhile"]);
         assert!(options.abbreviations.contains(&"approx.".to_string()));
         assert!(options.preserve_lowercase.contains(&"ffmpeg".to_string()));
-        assert_eq!(options.rules, RuleSet::ALL);
+        assert_eq!(options.rules, RuleSet::DEFAULT);
     }
 }