cargo-rail: fixing the command structure; final cleanup?

Author: loadingalias

src/commands/init.rs

@@ -22,15 +22,21 @@ pub fn run_init(
   let workspace_root = ctx.workspace_root();
   let config_path = workspace_root.join(output_path);
 
-  if let Some(existing) = check_existing_config(workspace_root) {
+  // Check if target path already exists
+  if config_path.exists() {
     if !force {
       return Err(RailError::with_help(
-        format!("configuration exists: {}", existing.display()),
+        format!("configuration exists: {}", config_path.display()),
         "use --force to overwrite",
       ));
     }
     if !check {
-      eprintln!("overwriting: {}", existing.display());
+      eprintln!("overwriting: {}", config_path.display());
+    }
+  } else if let Some(existing) = check_existing_config(workspace_root) {
+    // A config exists elsewhere - warn but allow writing to different path
+    if !check {
+      eprintln!("note: existing config at {}", existing.display());
     }
   }
 
@@ -143,15 +149,21 @@ pub fn run_init_standalone(
 ) -> RailResult<()> {
   let config_path = workspace_root.join(output_path);
 
-  if let Some(existing) = check_existing_config(workspace_root) {
+  // Check if target path already exists
+  if config_path.exists() {
     if !force {
       return Err(RailError::with_help(
-        format!("configuration exists: {}", existing.display()),
+        format!("configuration exists: {}", config_path.display()),
         "use --force to overwrite",
       ));
     }
     if !check {
-      eprintln!("overwriting: {}", existing.display());
+      eprintln!("overwriting: {}", config_path.display());
+    }
+  } else if let Some(existing) = check_existing_config(workspace_root) {
+    // A config exists elsewhere - warn but allow writing to different path
+    if !check {
+      eprintln!("note: existing config at {}", existing.display());
     }
   }
 

src/commands/release.rs

@@ -197,16 +197,11 @@ pub fn run_release_check(
 }
 
 /// Initialize release configuration
-pub fn run_release_init(ctx: &WorkspaceContext, crates: Option<&str>, check: bool) -> RailResult<()> {
+pub fn run_release_init(ctx: &WorkspaceContext, crates: Option<Vec<String>>, check: bool) -> RailResult<()> {
   use crate::config::{ChangelogConfig, CrateReleaseConfig, RailConfig};
   use std::fs;
 
-  let requested_crates: Option<Vec<String>> = crates.map(|s| {
-    s.split(',')
-      .map(|name| name.trim().to_string())
-      .filter(|name| !name.is_empty())
-      .collect()
-  });
+  let requested_crates = crates;
 
   let members = ctx.cargo.metadata().workspace_packages();
   let workspace_root = ctx.workspace_root();

src/commands/split.rs

@@ -107,16 +107,11 @@ pub fn run_split(
 }
 
 /// Initialize split configuration for crates
-pub fn run_split_init(ctx: &WorkspaceContext, crates: Option<&str>, check: bool) -> RailResult<()> {
+pub fn run_split_init(ctx: &WorkspaceContext, crates: Option<Vec<String>>, check: bool) -> RailResult<()> {
   use crate::config::RailConfig;
   use std::fs;
 
-  let requested_crates: Option<Vec<String>> = crates.map(|s| {
-    s.split(',')
-      .map(|name| name.trim().to_string())
-      .filter(|name| !name.is_empty())
-      .collect()
-  });
+  let requested_crates = crates;
 
   let splits = detect_workspace_splits(ctx, requested_crates.as_deref())?;
 

src/main.rs

@@ -30,7 +30,7 @@ enum Commands {
     #[arg(long)]
     since: Option<String>,
     /// Start ref (for SHA pair mode)
-    #[arg(long, conflicts_with = "since")]
+    #[arg(long, conflicts_with = "since", requires = "to")]
     from: Option<String>,
     /// End ref (for SHA pair mode)
     #[arg(long, requires = "from")]
@@ -125,8 +125,10 @@ enum Commands {
 
   /// Split a crate to a standalone repository with git history
   Split {
-    /// Crate name, or 'init' to configure
-    crate_name: Option<String>,
+    /// Action: 'init' to configure splits, or crate name to split
+    action: Option<String>,
+    /// Additional crate name(s) for init
+    crate_names: Vec<String>,
     /// Split all configured crates
     #[arg(short, long)]
     all: bool,
@@ -357,32 +359,26 @@ fn main() {
 
     // Split/Sync
     Commands::Split {
-      crate_name,
+      action,
+      crate_names,
       all,
       remote,
       check,
       format,
     } => {
-      // Check if this is 'split init <crates>'
-      if let Some(name) = crate_name {
-        if name == "init" {
-          // cargo rail split init (all crates)
-          commands::run_split_init(&ctx, None, check)
-        } else if name.starts_with("init,") || name.starts_with("init ") {
-          // cargo rail split "init,crate1,crate2" (specific crates)
-          let crates = name
-            .strip_prefix("init,")
-            .or_else(|| name.strip_prefix("init "))
-            .unwrap()
-            .trim();
-          commands::run_split_init(&ctx, Some(crates), check)
+      // Handle init subcommand: cargo rail split init [crate1 crate2 ...]
+      if action.as_deref() == Some("init") {
+        let crates = if crate_names.is_empty() {
+          None
         } else {
-          // cargo rail split mycrate (regular split)
-          commands::run_split(&ctx, Some(name.clone()), all, remote.clone(), check, format)
-        }
+          Some(crate_names)
+        };
+        commands::run_split_init(&ctx, crates, check)
       } else {
-        // cargo rail split --all or cargo rail split --check
-        commands::run_split(&ctx, None, all, remote.clone(), check, format)
+        // Regular split command
+        // If action is provided and not "init", treat it as the crate name
+        let crate_name = action;
+        commands::run_split(&ctx, crate_name, all, remote.clone(), check, format)
       }
     }
     Commands::Sync {
@@ -422,12 +418,12 @@ fn main() {
       // Handle init subcommand
       if action.as_deref() == Some("init") {
         // cargo rail release init <crates>
-        let crates_str = if crate_names.is_empty() {
+        let crates = if crate_names.is_empty() {
           None
         } else {
-          Some(crate_names.join(","))
+          Some(crate_names)
         };
-        commands::run_release_init(&ctx, crates_str.as_deref(), check)
+        commands::run_release_init(&ctx, crates, check)
       } else {
         // Regular release command
         // If action is provided and not "init", treat it as the first crate name

src/release/planner.rs

@@ -2,7 +2,7 @@
 
 use crate::config::{ChangelogRelativeTo, ReleaseConfig};
 use crate::error::{RailError, RailResult};
-use crate::release::version::{BumpType, VersionBumper};
+use crate::release::version::BumpType;
 use crate::workspace::WorkspaceContext;
 use semver::Version;
 use serde::Serialize;
@@ -127,8 +127,8 @@ impl<'a> ReleasePlanner<'a> {
 
     let manifest_path = package.manifest_path.clone().into_std_path_buf();
 
-    // Get current version
-    let current_version = VersionBumper::get_version(&manifest_path)?;
+    // Get current version from cargo_metadata (already resolves workspace inheritance)
+    let current_version = package.version.clone();
 
     // Calculate new version
     let new_version = bump_type.apply(&current_version);

src/release/version.rs

@@ -121,23 +121,52 @@ impl VersionBumper {
   /// Bump version in Cargo.toml
   ///
   /// Uses lossless TOML editing to preserve comments and formatting.
+  ///
+  /// # Errors
+  /// Returns an error if the crate uses workspace version inheritance.
   pub fn bump_version(manifest_path: &Path, bump_type: BumpType) -> RailResult<Version> {
     use crate::toml::editor::TomlEditor;
 
     let mut editor = TomlEditor::open(manifest_path)?;
     let doc = editor.doc();
 
-    // Get current version
-    let current_str = doc
-      .get("package")
-      .and_then(|p| p.get("version"))
-      .and_then(|v| v.as_str())
-      .ok_or_else(|| {
-        RailError::with_help(
-          format!("No version field in {}", manifest_path.display()),
-          "Ensure [package.version] is set",
-        )
-      })?;
+    // Get current version - check for workspace inheritance
+    let version_item = doc.get("package").and_then(|p| p.get("version"));
+
+    // Check if using workspace inheritance
+    if let Some(item) = version_item
+      && (item.is_inline_table() || item.is_table())
+    {
+      // Check for { workspace = true }
+      if let Some(tbl) = item.as_inline_table() {
+        if tbl.get("workspace").and_then(|v| v.as_bool()) == Some(true) {
+          return Err(RailError::with_help(
+            format!(
+              "Cannot bump version in {}: uses workspace inheritance",
+              manifest_path.display()
+            ),
+            "Set a specific version in this crate's Cargo.toml, or update [workspace.package.version] in the root Cargo.toml",
+          ));
+        }
+      } else if let Some(tbl) = item.as_table()
+        && tbl.get("workspace").and_then(|v| v.as_bool()) == Some(true)
+      {
+        return Err(RailError::with_help(
+          format!(
+            "Cannot bump version in {}: uses workspace inheritance",
+            manifest_path.display()
+          ),
+          "Set a specific version in this crate's Cargo.toml, or update [workspace.package.version] in the root Cargo.toml",
+        ));
+      }
+    }
+
+    let current_str = version_item.and_then(|v| v.as_str()).ok_or_else(|| {
+      RailError::with_help(
+        format!("No version field in {}", manifest_path.display()),
+        "Ensure [package.version] is set",
+      )
+    })?;
 
     let current = Version::parse(current_str).map_err(|e| {
       RailError::message(format!(