Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
finish
  • Loading branch information
psteinroe committed Jan 27, 2025
commit 8a215d3faf247b10d85caf2190e1af859d561156
6 changes: 4 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 8 additions & 10 deletions crates/pg_cli/src/execute/traverse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Comment on lines +459 to +465
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is the relevant part in can_handle

// handle:
// - directories
// - symlinks
Expand All @@ -476,15 +482,7 @@ impl TraversalContext for TraversalOptions<'_, '_> {
}

// bail on fifo and socket files
if !self.fs.path_is_file(path) {
return false;
}

// only allow .sql and .pg files for now
if path
.extension()
.is_none_or(|ext| ext != "sql" && ext != "pg")
{
if !is_valid_file {
return false;
}

Expand Down
2 changes: 0 additions & 2 deletions crates/pg_cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ impl<'app> CliSession<'app> {
staged,
changed,
since,
after,
} => run_command(
self,
&cli_options,
Expand All @@ -85,7 +84,6 @@ impl<'app> CliSession<'app> {
staged,
changed,
since,
after,
},
),
PgLspCommand::Clean => commands::clean::clean(self),
Expand Down
32 changes: 5 additions & 27 deletions crates/pg_configuration/src/migrations.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::str::FromStr;

use biome_deserialize_macros::{Merge, Partial};
use bpaf::Bpaf;
use serde::{Deserialize, Serialize};
Expand All @@ -10,30 +8,10 @@ use serde::{Deserialize, Serialize};
#[partial(serde(rename_all = "snake_case", default, deny_unknown_fields))]
pub struct MigrationsConfiguration {
/// The directory where the migration files are stored
#[partial(bpaf(hide))]
pub migration_dir: Option<String>,

/// The pattern used to store migration files
#[partial(bpaf(hide))]
pub migration_pattern: Option<MigrationPattern>,
}

#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, Merge)]
pub enum MigrationPattern {
/// The migration files are stored in the root of the migration directory
Root,
/// The migration files are stored in a subdirectory of the migration directory
Subdirectory,
}

impl FromStr for MigrationPattern {
type Err = String;
#[partial(bpaf(long("migrations-dir")))]
pub migrations_dir: String,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if we use bpaf-derive, but if we do, we could also parse into a PathBuf.

Examples of that were hard to find, so maybe that's not the recommended way – I'm not sure

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would be preferred, but biomes Merge does not work with PathBuf :(


fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"root" => Ok(Self::Root),
"subdirectory" => Ok(Self::Subdirectory),
_ => Err(format!("Invalid migration pattern: {}", s)),
}
}
/// Ignore any migrations before this timestamp
#[partial(bpaf(long("after")))]
pub after: u64,
}
1 change: 1 addition & 0 deletions crates/pg_workspace/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ tree-sitter.workspace = true
tree_sitter_sql.workspace = true

[dev-dependencies]
tempfile = "3.15.0"

[lib]
doctest = false
Expand Down
93 changes: 33 additions & 60 deletions crates/pg_workspace/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ use std::{
borrow::Cow,
num::NonZeroU64,
path::{Path, PathBuf},
str::FromStr,
sync::{RwLock, RwLockReadGuard, RwLockWriteGuard},
};

use ignore::gitignore::{Gitignore, GitignoreBuilder};
use pg_configuration::{
database::PartialDatabaseConfiguration,
diagnostics::InvalidIgnorePattern,
files::{FilesConfiguration, MigrationPattern},
migrations::{MigrationsConfiguration, PartialMigrationsConfiguration},
files::FilesConfiguration,
migrations::{self, MigrationsConfiguration, PartialMigrationsConfiguration},
ConfigurationDiagnostic, LinterConfiguration, PartialConfiguration,
};
use pg_fs::FileSystem;
Expand All @@ -32,7 +33,7 @@ pub struct Settings {
pub linter: LinterSettings,

/// Migrations settings
pub migrations: Option<Migrations>,
pub migrations: Option<MigrationSettings>,
}

#[derive(Debug)]
Expand Down Expand Up @@ -105,7 +106,13 @@ impl Settings {
to_linter_settings(working_directory.clone(), LinterConfiguration::from(linter))?;
}

// TODO migrations
// Migrations settings
if let Some(migrations) = configuration.migrations {
self.migrations = to_migration_settings(
working_directory.clone(),
MigrationsConfiguration::from(migrations),
);
}

Ok(())
}
Expand Down Expand Up @@ -313,68 +320,34 @@ pub struct FilesSettings {
}

/// Migration settings
#[derive(Debug)]
pub(crate) struct Migrations {
path: PathBuf,
pattern: MigrationPattern,
}

pub(crate) struct Migration {
timestamp: u64,
name: String,
#[derive(Debug, Default)]
pub struct MigrationSettings {
pub path: Option<PathBuf>,
pub after: Option<u64>,
}

impl Migrations {
fn get_migration(&self, path: &Path) -> Option<Migration> {
// check if path is a child of the migration directory
match path.canonicalize() {
Ok(canonical_child) => match self.path.canonicalize() {
Ok(canonical_dir) => canonical_child.starts_with(&canonical_dir),
Err(_) => return None,
},
Err(_) => return None,
};

match self.pattern {
// supabase style migrations/0001_create_table.sql
MigrationPattern::Root => {
let file_name = path.file_name()?.to_str()?;
let timestamp = file_name.split('_').next()?;
let name = file_name
.split('_')
.skip(1)
.collect::<Vec<&str>>()
.join("_");
let timestamp = timestamp.parse().ok()?;
Some(Migration { timestamp, name })
}
// drizzle / prisma style migrations/0001_create_table/migration.sql
MigrationPattern::Subdirectory => {
let relative_path = path.strip_prefix(&self.path).ok()?;
let components: Vec<_> = relative_path.components().collect();
if components.len() != 2 {
return None;
}
let dir_name = components[0].as_os_str().to_str()?;
let parts: Vec<&str> = dir_name.splitn(2, '_').collect();
if parts.len() != 2 {
return None;
}
let timestamp = parts[0].parse().ok()?;
let name = parts[1].to_string();
Some(Migration { timestamp, name })
}
impl From<PartialMigrationsConfiguration> for MigrationSettings {
fn from(value: PartialMigrationsConfiguration) -> Self {
Self {
path: value.migrations_dir.map(PathBuf::from),
after: value.after,
}
}
}

impl From<MigrationsConfiguration> for Migrations {
fn from(value: MigrationsConfiguration) -> Self {
Self {
path: value.migration_dir,
pattern: value.migration_pattern,
}
}
fn to_migration_settings(
working_directory: Option<PathBuf>,
conf: MigrationsConfiguration,
) -> Option<MigrationSettings> {
tracing::debug!(
"Migrations configuration: {:?}, dir: {:?}",
conf,
working_directory
);
working_directory.map(|working_directory| MigrationSettings {
path: Some(working_directory.join(conf.migrations_dir)),
after: Some(conf.after),
})
}

/// Limit the size of files to 1.0 MiB by default
Expand Down
29 changes: 27 additions & 2 deletions crates/pg_workspace/src/workspace/server.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
use std::{fs, future::Future, panic::RefUnwindSafe, path::Path, sync::RwLock};
use std::{
fs,
future::Future,
panic::RefUnwindSafe,
path::{Path, PathBuf},
sync::RwLock,
};

use analyser::AnalyserVisitorBuilder;
use change::StatementChange;
Expand Down Expand Up @@ -33,6 +39,7 @@ use super::{
mod analyser;
mod change;
mod document;
mod migration;
mod pg_query;
mod tree_sitter;

Expand Down Expand Up @@ -150,13 +157,31 @@ impl WorkspaceServer {
Ok(())
}

fn is_ignored_by_migration_config(&self, path: &Path) -> bool {
let set = self.settings();
set.as_ref()
.migrations
.as_ref()
.and_then(|migration_settings| {
tracing::info!("Checking migration settings: {:?}", migration_settings);
let ignore_before = migration_settings.after.as_ref()?;
tracing::info!("Checking migration settings: {:?}", ignore_before);
let migrations_dir = migration_settings.path.as_ref()?;
tracing::info!("Checking migration settings: {:?}", migrations_dir);
let migration = migration::get_migration(path, migrations_dir)?;

Some(&migration.timestamp <= ignore_before)
})
.unwrap_or(false)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if we pass a migrations_dir but no after? Wouldn't the and_then return None, so the function would always return false?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, and we do not want to ignore any migration file if no after is specified do we?

}

/// Check whether a file is ignored in the top-level config `files.ignore`/`files.include`
fn is_ignored(&self, path: &Path) -> bool {
let file_name = path.file_name().and_then(|s| s.to_str());
// Never ignore PGLSP's config file regardless `include`/`ignore`
(file_name != Some(ConfigName::pglsp_toml())) &&
// Apply top-level `include`/`ignore
(self.is_ignored_by_top_level_config(path))
(self.is_ignored_by_top_level_config(path) || self.is_ignored_by_migration_config(path))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_ignored is called by is_path_ignored and we add the new check here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very sweet!

}

/// Check whether a file is ignored in the top-level config `files.ignore`/`files.include`
Expand Down
Loading
Loading