From c46336a77bfe8cc6bd5f44f6b7545967aaaa78e8 Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Fri, 23 Jan 2026 14:44:07 +0900 Subject: [PATCH] Fix unicode, enum default --- .../changepack_log_xBpCqklmvkjyqCv1XnebC.json | 1 + Cargo.lock | 20 +-- crates/vespertide-cli/src/commands/diff.rs | 7 +- .../vespertide-cli/src/commands/revision.rs | 115 +++++++++++++++--- crates/vespertide-core/src/action.rs | 4 +- crates/vespertide-core/src/schema/column.rs | 15 +++ crates/vespertide-planner/src/validate.rs | 4 + .../src/sql/modify_column_default.rs | 17 ++- .../src/sql/modify_column_type.rs | 12 +- 9 files changed, 161 insertions(+), 34 deletions(-) create mode 100644 .changepacks/changepack_log_xBpCqklmvkjyqCv1XnebC.json diff --git a/.changepacks/changepack_log_xBpCqklmvkjyqCv1XnebC.json b/.changepacks/changepack_log_xBpCqklmvkjyqCv1XnebC.json new file mode 100644 index 0000000..f565b49 --- /dev/null +++ b/.changepacks/changepack_log_xBpCqklmvkjyqCv1XnebC.json @@ -0,0 +1 @@ +{"changes":{"crates/vespertide-macro/Cargo.toml":"Patch","crates/vespertide-naming/Cargo.toml":"Patch","crates/vespertide-core/Cargo.toml":"Patch","crates/vespertide-cli/Cargo.toml":"Patch","crates/vespertide-planner/Cargo.toml":"Patch","crates/vespertide-loader/Cargo.toml":"Patch","crates/vespertide-query/Cargo.toml":"Patch","crates/vespertide/Cargo.toml":"Patch","crates/vespertide-config/Cargo.toml":"Patch","crates/vespertide-exporter/Cargo.toml":"Patch"},"note":"Fix unicode issue and enum default issue","date":"2026-01-23T05:43:59.168949900Z"} \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index cfffdca..a2ec8ac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2995,7 +2995,7 @@ checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" [[package]] name = "vespertide" -version = "0.1.32" +version = "0.1.33" dependencies = [ "vespertide-core", "vespertide-macro", @@ -3003,7 +3003,7 @@ dependencies = [ [[package]] name = "vespertide-cli" -version = "0.1.32" +version = "0.1.33" dependencies = [ "anyhow", "assert_cmd", @@ -3028,7 +3028,7 @@ dependencies = [ [[package]] name = "vespertide-config" -version = "0.1.32" +version = "0.1.33" dependencies = [ "clap", "schemars", @@ -3038,7 +3038,7 @@ dependencies = [ [[package]] name = "vespertide-core" -version = "0.1.32" +version = "0.1.33" dependencies = [ "rstest", "schemars", @@ -3050,7 +3050,7 @@ dependencies = [ [[package]] name = "vespertide-exporter" -version = "0.1.32" +version = "0.1.33" dependencies = [ "insta", "rstest", @@ -3061,7 +3061,7 @@ dependencies = [ [[package]] name = "vespertide-loader" -version = "0.1.32" +version = "0.1.33" dependencies = [ "anyhow", "rstest", @@ -3076,7 +3076,7 @@ dependencies = [ [[package]] name = "vespertide-macro" -version = "0.1.32" +version = "0.1.33" dependencies = [ "proc-macro2", "quote", @@ -3093,11 +3093,11 @@ dependencies = [ [[package]] name = "vespertide-naming" -version = "0.1.32" +version = "0.1.33" [[package]] name = "vespertide-planner" -version = "0.1.32" +version = "0.1.33" dependencies = [ "insta", "rstest", @@ -3108,7 +3108,7 @@ dependencies = [ [[package]] name = "vespertide-query" -version = "0.1.32" +version = "0.1.33" dependencies = [ "insta", "rstest", diff --git a/crates/vespertide-cli/src/commands/diff.rs b/crates/vespertide-cli/src/commands/diff.rs index 058232e..df34a97 100644 --- a/crates/vespertide-cli/src/commands/diff.rs +++ b/crates/vespertide-cli/src/commands/diff.rs @@ -126,8 +126,11 @@ fn format_action(action: &MigrationAction) -> String { new_comment, } => { let comment_display = new_comment.as_deref().unwrap_or("(none)"); - let truncated = if comment_display.len() > 30 { - format!("{}...", &comment_display[..27]) + let truncated = if comment_display.chars().count() > 30 { + format!( + "{}...", + comment_display.chars().take(27).collect::() + ) } else { comment_display.to_string() }; diff --git a/crates/vespertide-cli/src/commands/revision.rs b/crates/vespertide-cli/src/commands/revision.rs index 5a4ee22..db1d449 100644 --- a/crates/vespertide-cli/src/commands/revision.rs +++ b/crates/vespertide-cli/src/commands/revision.rs @@ -4,7 +4,7 @@ use std::fs; use anyhow::{Context, Result}; use chrono::Utc; use colored::Colorize; -use dialoguer::Input; +use dialoguer::{Input, Select}; use serde_json::Value; use vespertide_config::FileFormat; use vespertide_core::{MigrationAction, MigrationPlan}; @@ -118,15 +118,32 @@ fn prompt_fill_with_value(prompt: &str) -> Result { Ok(wrap_if_spaces(value)) } +/// Prompt the user to select an enum value using dialoguer Select. +/// Returns the selected value wrapped in single quotes for SQL. +#[cfg(not(tarpaulin_include))] +fn prompt_enum_value(prompt: &str, enum_values: &[String]) -> Result { + let selection = Select::new() + .with_prompt(prompt) + .items(enum_values) + .default(0) + .interact() + .context("failed to read selection")?; + // Return the selected value with single quotes for SQL enum literal + Ok(format!("'{}'", enum_values[selection])) +} + /// Collect fill_with values interactively for missing columns. /// The `prompt_fn` parameter allows injecting a mock for testing. -fn collect_fill_with_values( +/// The `enum_prompt_fn` parameter handles enum type columns with selection UI. +fn collect_fill_with_values( missing: &[vespertide_planner::FillWithRequired], fill_values: &mut HashMap<(String, String), String>, prompt_fn: F, + enum_prompt_fn: E, ) -> Result<()> where F: Fn(&str) -> Result, + E: Fn(&str, &[String]) -> Result, { print_fill_with_header(); @@ -139,7 +156,13 @@ where item.action_type, ); - let value = prompt_fn(&prompt)?; + let value = if let Some(enum_values) = &item.enum_values { + // Use selection UI for enum types + enum_prompt_fn(&prompt, enum_values)? + } else { + // Use text input for other types + prompt_fn(&prompt)? + }; fill_values.insert((item.table.clone(), item.column.clone()), value); } @@ -184,18 +207,20 @@ fn apply_fill_with_to_plan( /// Handle interactive fill_with collection if there are missing values. /// Returns the updated fill_values map after collecting from user. -fn handle_missing_fill_with( +fn handle_missing_fill_with( plan: &mut MigrationPlan, fill_values: &mut HashMap<(String, String), String>, prompt_fn: F, + enum_prompt_fn: E, ) -> Result<()> where F: Fn(&str) -> Result, + E: Fn(&str, &[String]) -> Result, { let missing = find_missing_fill_with(plan); if !missing.is_empty() { - collect_fill_with_values(&missing, fill_values, prompt_fn)?; + collect_fill_with_values(&missing, fill_values, prompt_fn, enum_prompt_fn)?; // Apply the collected fill_with values apply_fill_with_to_plan(plan, fill_values); @@ -228,7 +253,12 @@ pub fn cmd_revision(message: String, fill_with_args: Vec) -> Result<()> apply_fill_with_to_plan(&mut plan, &fill_values); // Handle any missing fill_with values interactively - handle_missing_fill_with(&mut plan, &mut fill_values, prompt_fill_with_value)?; + handle_missing_fill_with( + &mut plan, + &mut fill_values, + prompt_fill_with_value, + prompt_enum_value, + )?; plan.comment = Some(message); if plan.created_at.is_none() { @@ -838,6 +868,11 @@ mod tests { print_fill_with_footer(); } + // Mock enum prompt function for tests - returns first enum value quoted + fn mock_enum_prompt(_prompt: &str, values: &[String]) -> Result { + Ok(format!("'{}'", values[0])) + } + #[test] fn test_collect_fill_with_values_single_item() { use vespertide_planner::FillWithRequired; @@ -849,6 +884,7 @@ mod tests { action_type: "AddColumn", column_type: Some("text".to_string()), default_value: Some("''".to_string()), + enum_values: None, }]; let mut fill_values = HashMap::new(); @@ -857,7 +893,8 @@ mod tests { let mock_prompt = |_prompt: &str| -> Result { Ok("'test@example.com'".to_string()) }; - let result = collect_fill_with_values(&missing, &mut fill_values, mock_prompt); + let result = + collect_fill_with_values(&missing, &mut fill_values, mock_prompt, mock_enum_prompt); assert!(result.is_ok()); assert_eq!(fill_values.len(), 1); assert_eq!( @@ -878,6 +915,7 @@ mod tests { action_type: "AddColumn", column_type: Some("text".to_string()), default_value: Some("''".to_string()), + enum_values: None, }, FillWithRequired { action_index: 1, @@ -886,6 +924,7 @@ mod tests { action_type: "ModifyColumnNullable", column_type: None, default_value: None, + enum_values: None, }, ]; @@ -903,7 +942,8 @@ mod tests { } }; - let result = collect_fill_with_values(&missing, &mut fill_values, mock_prompt); + let result = + collect_fill_with_values(&missing, &mut fill_values, mock_prompt, mock_enum_prompt); assert!(result.is_ok()); assert_eq!(fill_values.len(), 2); assert_eq!( @@ -930,7 +970,8 @@ mod tests { // Note: The function still prints header/footer even for empty list // This is a design choice - in practice, cmd_revision won't call this with empty list - let result = collect_fill_with_values(&missing, &mut fill_values, mock_prompt); + let result = + collect_fill_with_values(&missing, &mut fill_values, mock_prompt, mock_enum_prompt); assert!(result.is_ok()); assert!(fill_values.is_empty()); } @@ -946,6 +987,7 @@ mod tests { action_type: "AddColumn", column_type: Some("text".to_string()), default_value: Some("''".to_string()), + enum_values: None, }]; let mut fill_values = HashMap::new(); @@ -954,7 +996,8 @@ mod tests { let mock_prompt = |_prompt: &str| -> Result { Err(anyhow::anyhow!("input cancelled")) }; - let result = collect_fill_with_values(&missing, &mut fill_values, mock_prompt); + let result = + collect_fill_with_values(&missing, &mut fill_values, mock_prompt, mock_enum_prompt); assert!(result.is_err()); assert!(fill_values.is_empty()); } @@ -998,7 +1041,8 @@ mod tests { let mock_prompt = |_prompt: &str| -> Result { Ok("'test@example.com'".to_string()) }; - let result = handle_missing_fill_with(&mut plan, &mut fill_values, mock_prompt); + let result = + handle_missing_fill_with(&mut plan, &mut fill_values, mock_prompt, mock_enum_prompt); assert!(result.is_ok()); // Verify fill_with was applied to the plan @@ -1049,7 +1093,8 @@ mod tests { panic!("Should not be called when no missing fill_with values"); }; - let result = handle_missing_fill_with(&mut plan, &mut fill_values, mock_prompt); + let result = + handle_missing_fill_with(&mut plan, &mut fill_values, mock_prompt, mock_enum_prompt); assert!(result.is_ok()); assert!(fill_values.is_empty()); } @@ -1085,7 +1130,8 @@ mod tests { let mock_prompt = |_prompt: &str| -> Result { Err(anyhow::anyhow!("user cancelled")) }; - let result = handle_missing_fill_with(&mut plan, &mut fill_values, mock_prompt); + let result = + handle_missing_fill_with(&mut plan, &mut fill_values, mock_prompt, mock_enum_prompt); assert!(result.is_err()); // Plan should not be modified on error @@ -1144,7 +1190,8 @@ mod tests { } }; - let result = handle_missing_fill_with(&mut plan, &mut fill_values, mock_prompt); + let result = + handle_missing_fill_with(&mut plan, &mut fill_values, mock_prompt, mock_enum_prompt); assert!(result.is_ok()); // Verify both actions were updated @@ -1163,6 +1210,46 @@ mod tests { } } + #[test] + fn test_collect_fill_with_values_enum_column() { + use vespertide_planner::FillWithRequired; + + let missing = vec![FillWithRequired { + action_index: 0, + table: "orders".to_string(), + column: "status".to_string(), + action_type: "AddColumn", + column_type: Some("enum".to_string()), + default_value: None, + enum_values: Some(vec![ + "pending".to_string(), + "confirmed".to_string(), + "shipped".to_string(), + ]), + }]; + + let mut fill_values = HashMap::new(); + + // Mock prompt function that should NOT be called for enum columns + let mock_prompt = |_prompt: &str| -> Result { + panic!("Should not be called for enum columns"); + }; + + // Mock enum prompt that selects the second value + let mock_enum = |_prompt: &str, values: &[String]| -> Result { + // Select "confirmed" (index 1) + Ok(format!("'{}'", values[1])) + }; + + let result = collect_fill_with_values(&missing, &mut fill_values, mock_prompt, mock_enum); + assert!(result.is_ok()); + assert_eq!(fill_values.len(), 1); + assert_eq!( + fill_values.get(&("orders".to_string(), "status".to_string())), + Some(&"'confirmed'".to_string()) + ); + } + #[test] fn test_wrap_if_spaces_empty() { assert_eq!(wrap_if_spaces("".to_string()), ""); diff --git a/crates/vespertide-core/src/action.rs b/crates/vespertide-core/src/action.rs index d8a4ecf..f8c9e97 100644 --- a/crates/vespertide-core/src/action.rs +++ b/crates/vespertide-core/src/action.rs @@ -252,8 +252,8 @@ impl fmt::Display for MigrationAction { new_comment, } => { if let Some(comment) = new_comment { - let display = if comment.len() > 30 { - format!("{}...", &comment[..27]) + let display = if comment.chars().count() > 30 { + format!("{}...", comment.chars().take(27).collect::()) } else { comment.clone() }; diff --git a/crates/vespertide-core/src/schema/column.rs b/crates/vespertide-core/src/schema/column.rs index fc47514..3b39f75 100644 --- a/crates/vespertide-core/src/schema/column.rs +++ b/crates/vespertide-core/src/schema/column.rs @@ -126,6 +126,21 @@ impl ColumnType { ColumnType::Complex(ty) => ty.default_fill_value(), } } + + /// Get enum variant names if this is an enum type + /// Returns None if not an enum, Some(names) otherwise + pub fn enum_variant_names(&self) -> Option> { + match self { + ColumnType::Complex(ComplexColumnType::Enum { values, .. }) => Some( + values + .variant_names() + .into_iter() + .map(String::from) + .collect(), + ), + _ => None, + } + } } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] diff --git a/crates/vespertide-planner/src/validate.rs b/crates/vespertide-planner/src/validate.rs index 56bede7..0e0d35a 100644 --- a/crates/vespertide-planner/src/validate.rs +++ b/crates/vespertide-planner/src/validate.rs @@ -404,6 +404,8 @@ pub struct FillWithRequired { pub column_type: Option, /// Default fill value hint for this column type. pub default_value: Option, + /// Enum values if the column is an enum type (for selection UI). + pub enum_values: Option>, } /// Find all actions in a migration plan that require fill_with values. @@ -427,6 +429,7 @@ pub fn find_missing_fill_with(plan: &MigrationPlan) -> Vec { action_type: "AddColumn", column_type: Some(column.r#type.to_display_string()), default_value: column.r#type.default_fill_value().map(String::from), + enum_values: column.r#type.enum_variant_names(), }); } } @@ -445,6 +448,7 @@ pub fn find_missing_fill_with(plan: &MigrationPlan) -> Vec { action_type: "ModifyColumnNullable", column_type: None, default_value: None, + enum_values: None, // We don't have column type info here }); } } diff --git a/crates/vespertide-query/src/sql/modify_column_default.rs b/crates/vespertide-query/src/sql/modify_column_default.rs index 6e955dd..91c497d 100644 --- a/crates/vespertide-query/src/sql/modify_column_default.rs +++ b/crates/vespertide-query/src/sql/modify_column_default.rs @@ -3,7 +3,7 @@ use sea_query::{Alias, Query, Table}; use vespertide_core::{ColumnDef, TableDef}; use super::create_table::build_create_table_for_backend; -use super::helpers::build_sea_column_def_with_table; +use super::helpers::{build_sea_column_def_with_table, normalize_enum_default}; use super::rename_table::build_rename_table; use super::types::{BuiltQuery, DatabaseBackend, RawSql}; use crate::error::QueryError; @@ -21,9 +21,22 @@ pub fn build_modify_column_default( match backend { DatabaseBackend::Postgres => { let alter_sql = if let Some(default_value) = new_default { + // Look up column type to properly quote enum defaults + let column_type = current_schema + .iter() + .find(|t| t.name == table) + .and_then(|t| t.columns.iter().find(|c| c.name == column)) + .map(|c| &c.r#type); + + let normalized_default = if let Some(col_type) = column_type { + normalize_enum_default(col_type, default_value) + } else { + default_value.to_string() + }; + format!( "ALTER TABLE \"{}\" ALTER COLUMN \"{}\" SET DEFAULT {}", - table, column, default_value + table, column, normalized_default ) } else { format!( diff --git a/crates/vespertide-query/src/sql/modify_column_type.rs b/crates/vespertide-query/src/sql/modify_column_type.rs index 99924c3..7fea5d1 100644 --- a/crates/vespertide-query/src/sql/modify_column_type.rs +++ b/crates/vespertide-query/src/sql/modify_column_type.rs @@ -5,6 +5,7 @@ use vespertide_core::{ColumnType, ComplexColumnType, TableDef}; use super::create_table::build_create_table_for_backend; use super::helpers::{ apply_column_type_with_table, build_create_enum_type_sql, convert_default_for_backend, + normalize_enum_default, }; use super::rename_table::build_rename_table; use super::types::{BuiltQuery, DatabaseBackend}; @@ -186,12 +187,13 @@ pub fn build_modify_column_type( // 6. Restore DEFAULT if it existed if let Some(default_value) = column_default { + // Use normalize_enum_default to properly quote enum values + let normalized_default = + normalize_enum_default(new_type, &default_value.to_sql()); queries.push(BuiltQuery::Raw(super::types::RawSql::per_backend( format!( "ALTER TABLE \"{}\" ALTER COLUMN \"{}\" SET DEFAULT {}", - table, - column, - default_value.to_sql() + table, column, normalized_default ), String::new(), String::new(), @@ -241,7 +243,9 @@ pub fn build_modify_column_type( if let Some(default) = &column_def.default { let default_str = default.to_sql(); let converted = convert_default_for_backend(&default_str, backend); - col.default(sea_query::Expr::cust(converted)); + // Normalize enum default values if new type is an enum + let final_default = normalize_enum_default(new_type, &converted); + col.default(sea_query::Expr::cust(final_default)); } }