cli ergonomics (#172)

* initial commit * save * progress * finish * cleanup * cleanup * pr review

Author: psteinroe

Cargo.lock

@@ -12,31 +12,33 @@ version = "0.0.0"
 
 
 [dependencies]
-anyhow = { workspace = true }
-bpaf = { workspace = true, features = ["bright-color"] }
-crossbeam = { workspace = true }
-dashmap = "5.5.3"
-hdrhistogram = { version = "7.5.4", default-features = false }
-path-absolutize = { version = "3.1.1", optional = false, features = ["use_unix_paths_on_wasm"] }
-pg_analyse = { workspace = true }
-pg_configuration = { workspace = true }
-pg_console = { workspace = true }
-pg_diagnostics = { workspace = true }
-pg_flags = { workspace = true }
-pg_fs = { workspace = true }
-pg_lsp = { workspace = true }
-pg_text_edit = { workspace = true }
-pg_workspace = { workspace = true }
-quick-junit = "0.5.0"
-rayon = { workspace = true }
-rustc-hash = { workspace = true }
-serde = { workspace = true, features = ["derive"] }
-serde_json = { workspace = true }
-tokio = { workspace = true, features = ["io-std", "io-util", "net", "time", "rt", "sync", "rt-multi-thread", "macros"] }
-tracing = { workspace = true }
-tracing-appender = "0.2.3"
-tracing-subscriber = { workspace = true, features = ["env-filter", "json"] }
-tracing-tree = "0.4.0"
+anyhow = { workspace = true }
+biome_deserialize = { workspace = true }
+biome_deserialize_macros = { workspace = true }
+bpaf = { workspace = true, features = ["bright-color"] }
+crossbeam = { workspace = true }
+dashmap = "5.5.3"
+hdrhistogram = { version = "7.5.4", default-features = false }
+path-absolutize = { version = "3.1.1", optional = false, features = ["use_unix_paths_on_wasm"] }
+pg_analyse = { workspace = true }
+pg_configuration = { workspace = true }
+pg_console = { workspace = true }
+pg_diagnostics = { workspace = true }
+pg_flags = { workspace = true }
+pg_fs = { workspace = true }
+pg_lsp = { workspace = true }
+pg_text_edit = { workspace = true }
+pg_workspace = { workspace = true }
+quick-junit = "0.5.0"
+rayon = { workspace = true }
+rustc-hash = { workspace = true }
+serde = { workspace = true, features = ["derive"] }
+serde_json = { workspace = true }
+tokio = { workspace = true, features = ["io-std", "io-util", "net", "time", "rt", "sync", "rt-multi-thread", "macros"] }
+tracing = { workspace = true }
+tracing-appender = "0.2.3"
+tracing-subscriber = { workspace = true, features = ["env-filter", "json"] }
+tracing-tree = "0.4.0"
 
 [target.'cfg(unix)'.dependencies]
 libc = "0.2.161"

crates/pg_cli/Cargo.toml

@@ -1,5 +1,6 @@
 use crate::cli_options::CliOptions;
 use crate::{CliDiagnostic, Execution, TraversalMode};
+use biome_deserialize::Merge;
 use pg_configuration::PartialConfiguration;
 use pg_console::Console;
 use pg_fs::FileSystem;
@@ -9,9 +10,6 @@ use std::ffi::OsString;
 use super::{get_files_to_process_with_cli_options, CommandRunner};
 
 pub(crate) struct CheckCommandPayload {
- pub(crate) write: bool,
- pub(crate) fix: bool,
- pub(crate) unsafe_: bool,
  pub(crate) configuration: Option<PartialConfiguration>,
  pub(crate) paths: Vec<OsString>,
  pub(crate) stdin_file_path: Option<String>,
@@ -26,12 +24,20 @@ impl CommandRunner for CheckCommandPayload {
  fn merge_configuration(
  &mut self,
  loaded_configuration: LoadedConfiguration,
- fs: &DynRef<'_, dyn FileSystem>,
- console: &mut dyn Console,
+ _fs: &DynRef<'_, dyn FileSystem>,
+ _console: &mut dyn Console,
  ) -> Result<PartialConfiguration, WorkspaceError> {
- let LoadedConfiguration { configuration, .. } = loaded_configuration;
+ let LoadedConfiguration {
+ configuration: mut fs_configuration,
+ ..
+ } = loaded_configuration;
+
+ if let Some(configuration) = self.configuration.clone() {
+ // overwrite fs config with cli args
+ fs_configuration.merge_with(configuration);
+ }
 
- Ok(configuration)
+ Ok(fs_configuration)
  }
 
  fn get_files_to_process(
@@ -56,7 +62,7 @@ impl CommandRunner for CheckCommandPayload {
  }
 
  fn should_write(&self) -> bool {
- self.write || self.fix
+ false
  }
 
  fn get_execution(

crates/pg_cli/src/commands/check.rs

@@ -11,7 +11,7 @@ use pg_console::Console;
 use pg_fs::FileSystem;
 use pg_workspace::configuration::{load_configuration, LoadedConfiguration};
 use pg_workspace::settings::PartialConfigurationExt;
-use pg_workspace::workspace::{FixFileMode, UpdateSettingsParams};
+use pg_workspace::workspace::UpdateSettingsParams;
 use pg_workspace::{DynRef, Workspace, WorkspaceError};
 use std::ffi::OsString;
 use std::path::PathBuf;
@@ -33,18 +33,6 @@ pub enum PgLspCommand {
  /// Runs everything to the requested files.
  #[bpaf(command)]
  Check {
- /// Writes safe fixes, formatting and import sorting
- #[bpaf(long("write"), switch)]
- write: bool,
-
- /// Allow to do unsafe fixes, should be used with `--write` or `--fix`
- #[bpaf(long("unsafe"), switch)]
- unsafe_: bool,
-
- /// Alias for `--write`, writes safe fixes, formatting and import sorting
- #[bpaf(long("fix"), switch, hide_usage)]
- fix: bool,
-
  #[bpaf(external(partial_configuration), hide_usage, optional)]
  configuration: Option<PartialConfiguration>,
  #[bpaf(external, hide_usage)]
@@ -401,165 +389,9 @@ fn get_files_to_process_with_cli_options(
  }
 }
 
-/// Holds the options to determine the fix file mode.
-pub(crate) struct FixFileModeOptions {
- write: bool,
- suppress: bool,
- suppression_reason: Option<String>,
- fix: bool,
- unsafe_: bool,
-}
-
-/// - [Result]: if the given options are incompatible
-/// - [Option]: if no fixes are requested
-/// - [FixFileMode]: if safe or unsafe fixes are requested
-pub(crate) fn determine_fix_file_mode(
- options: FixFileModeOptions,
- console: &mut dyn Console,
-) -> Result<Option<FixFileMode>, CliDiagnostic> {
- let FixFileModeOptions {
- write,
- fix,
- suppress,
- suppression_reason: _,
- unsafe_,
- } = options;
-
- check_fix_incompatible_arguments(options)?;
-
- let safe_fixes = write || fix;
- let unsafe_fixes = (write || safe_fixes) && unsafe_;
-
- if unsafe_fixes {
- Ok(Some(FixFileMode::SafeAndUnsafeFixes))
- } else if safe_fixes {
- Ok(Some(FixFileMode::SafeFixes))
- } else if suppress {
- Ok(Some(FixFileMode::ApplySuppressions))
- } else {
- Ok(None)
- }
-}
-
-/// Checks if the fix file options are incompatible.
-fn check_fix_incompatible_arguments(options: FixFileModeOptions) -> Result<(), CliDiagnostic> {
- let FixFileModeOptions {
- write,
- suppress,
- suppression_reason,
- fix,
- ..
- } = options;
- if write && fix {
- return Err(CliDiagnostic::incompatible_arguments("--write", "--fix"));
- } else if suppress && write {
- return Err(CliDiagnostic::incompatible_arguments(
- "--suppress",
- "--write",
- ));
- } else if suppress && fix {
- return Err(CliDiagnostic::incompatible_arguments("--suppress", "--fix"));
- } else if !suppress && suppression_reason.is_some() {
- return Err(CliDiagnostic::unexpected_argument(
- "--reason",
- "`--reason` is only valid when `--suppress` is used.",
- ));
- };
- Ok(())
-}
-
 #[cfg(test)]
 mod tests {
  use super::*;
- use pg_console::BufferConsole;
-
- #[test]
- fn incompatible_arguments() {
- {
- let (write, suppress, suppression_reason, fix, unsafe_) =
- (true, false, None, true, false);
- assert!(check_fix_incompatible_arguments(FixFileModeOptions {
- write,
- suppress,
- suppression_reason,
- fix,
- unsafe_
- })
- .is_err());
- }
- }
-
- #[test]
- fn safe_fixes() {
- let mut console = BufferConsole::default();
-
- for (write, suppress, suppression_reason, fix, unsafe_) in [
- (true, false, None, false, false), // --write
- (false, false, None, true, false), // --fix
- ] {
- assert_eq!(
- determine_fix_file_mode(
- FixFileModeOptions {
- write,
- suppress,
- suppression_reason,
- fix,
- unsafe_
- },
- &mut console
- )
- .unwrap(),
- Some(FixFileMode::SafeFixes)
- );
- }
- }
-
- #[test]
- fn safe_and_unsafe_fixes() {
- let mut console = BufferConsole::default();
-
- for (write, suppress, suppression_reason, fix, unsafe_) in [
- (true, false, None, false, true), // --write --unsafe
- (false, false, None, true, true), // --fix --unsafe
- ] {
- assert_eq!(
- determine_fix_file_mode(
- FixFileModeOptions {
- write,
- suppress,
- suppression_reason,
- fix,
- unsafe_
- },
- &mut console
- )
- .unwrap(),
- Some(FixFileMode::SafeAndUnsafeFixes)
- );
- }
- }
-
- #[test]
- fn no_fix() {
- let mut console = BufferConsole::default();
-
- let (write, suppress, suppression_reason, fix, unsafe_) =
- (false, false, None, false, false);
- assert_eq!(
- determine_fix_file_mode(
- FixFileModeOptions {
- write,
- suppress,
- suppression_reason,
- fix,
- unsafe_
- },
- &mut console
- )
- .unwrap(),
- None
- );
- }
 
  /// Tests that all CLI options adhere to the invariants expected by `bpaf`.
  #[test]

crates/pg_cli/src/commands/mod.rs

@@ -456,7 +456,13 @@ impl TraversalContext for TraversalOptions<'_, '_> {
 
  fn can_handle(&self, pglsp_path: &PgLspPath) -> bool {
  let path = pglsp_path.as_path();
- if self.fs.path_is_dir(path) || self.fs.path_is_symlink(path) {
+
+ let is_valid_file = self.fs.path_is_file(path)
+ && path
+ .extension()
+ .is_some_and(|ext| ext == "sql" || ext == "pg");
+
+ if self.fs.path_is_dir(path) || self.fs.path_is_symlink(path) || is_valid_file {
  // handle:
  // - directories
  // - symlinks
@@ -476,7 +482,7 @@ impl TraversalContext for TraversalOptions<'_, '_> {
  }
 
  // bail on fifo and socket files
- if !self.fs.path_is_file(path) {
+ if !is_valid_file {
  return false;
  }
 

crates/pg_cli/src/execute/traverse.rs

@@ -67,9 +67,6 @@ impl<'app> CliSession<'app> {
  let result = match command {
  PgLspCommand::Version(_) => commands::version::full_version(self),
  PgLspCommand::Check {
- write,
- fix,
- unsafe_,
  cli_options,
  configuration,
  paths,
@@ -81,9 +78,6 @@ impl<'app> CliSession<'app> {
  self,
  &cli_options,
  CheckCommandPayload {
- write,
- fix,
- unsafe_,
  configuration,
  paths,
  stdin_file_path,

crates/pg_cli/src/lib.rs

@@ -1,10 +1,10 @@
-use biome_deserialize_macros::Partial;
+use biome_deserialize_macros::{Merge, Partial};
 use bpaf::Bpaf;
 use serde::{Deserialize, Serialize};
 
 /// The configuration of the database connection.
 #[derive(Clone, Debug, Deserialize, Eq, Partial, PartialEq, Serialize)]
-#[partial(derive(Bpaf, Clone, Eq, PartialEq))]
+#[partial(derive(Bpaf, Clone, Eq, PartialEq, Merge))]
 #[partial(serde(rename_all = "snake_case", default, deny_unknown_fields))]
 pub struct DatabaseConfiguration {
  /// The host of the database.