Skip to content

Commit c39d3b8

Browse files
authored
fix: address wasm pr feedback (#666)
1 parent c68c674 commit c39d3b8

12 files changed

Lines changed: 134 additions & 220 deletions

File tree

crates/pgls_cli/src/commands/mod.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,8 @@ pub enum PgLSCommand {
158158
/// Cleans the logs emitted by the daemon.
159159
Clean,
160160

161-
/// Exports the database schema to a JSON file for use with WASM bindings.
161+
/// Exports the database schema to JSON for use with WASM bindings.
162+
/// Writes to stdout by default, or to a file if --output is specified.
162163
#[bpaf(command("schema-export"))]
163164
SchemaExport {
164165
/// PostgreSQL connection string (e.g., postgres://user:pass@host/db)
@@ -170,14 +171,9 @@ pub enum PgLSCommand {
170171
)]
171172
connection_string: String,
172173

173-
/// Output file path for the JSON schema
174-
#[bpaf(
175-
long("output"),
176-
short('o'),
177-
argument("PATH"),
178-
fallback(PathBuf::from("schema.json"))
179-
)]
180-
output: PathBuf,
174+
/// Output file path for the JSON schema (defaults to stdout if not specified)
175+
#[bpaf(long("output"), short('o'), argument("PATH"))]
176+
output: Option<PathBuf>,
181177
},
182178

183179
#[bpaf(command("__run_server"), hide)]
Lines changed: 63 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,36 @@
11
//! Schema export command for WASM bindings.
22
//!
33
//! This command connects to a PostgreSQL database and exports the schema cache
4-
//! as a JSON file that can be used with the WASM bindings.
4+
//! as JSON that can be used with the WASM bindings.
55
66
use pgls_console::{ConsoleExt, EnvConsole, markup};
77
use pgls_schema_cache::SchemaCache;
88
use sqlx::postgres::PgPoolOptions;
9+
use std::io::Write;
910
use std::path::Path;
1011

1112
use crate::CliDiagnostic;
1213

13-
/// Export the database schema to a JSON file.
14+
/// Export the database schema to JSON.
1415
///
1516
/// # Arguments
1617
/// * `connection_string` - PostgreSQL connection string
17-
/// * `output_path` - Path to write the JSON output
18+
/// * `output_path` - Optional path to write the JSON output (stdout if None)
1819
pub async fn run_schema_export(
1920
connection_string: &str,
20-
output_path: &Path,
21+
output_path: Option<&Path>,
2122
) -> Result<(), CliDiagnostic> {
2223
let mut console = EnvConsole::default();
2324

24-
console.log(markup! {
25-
"Connecting to database..."
26-
});
25+
// Only print progress to stderr if we're writing to stdout,
26+
// so the JSON output is clean and can be piped
27+
let write_to_stdout = output_path.is_none();
28+
29+
if !write_to_stdout {
30+
console.log(markup! {
31+
"Connecting to database..."
32+
});
33+
}
2734

2835
// Connect to the database
2936
let pool = PgPoolOptions::new()
@@ -36,9 +43,11 @@ pub async fn run_schema_export(
3643
)))
3744
})?;
3845

39-
console.log(markup! {
40-
"Loading schema cache..."
41-
});
46+
if !write_to_stdout {
47+
console.log(markup! {
48+
"Loading schema cache..."
49+
});
50+
}
4251

4352
// Load the schema cache
4453
let schema_cache = SchemaCache::load(&pool).await.map_err(|e| {
@@ -47,9 +56,11 @@ pub async fn run_schema_export(
4756
)))
4857
})?;
4958

50-
console.log(markup! {
51-
"Serializing schema..."
52-
});
59+
if !write_to_stdout {
60+
console.log(markup! {
61+
"Serializing schema..."
62+
});
63+
}
5364

5465
// Serialize to JSON
5566
let json = serde_json::to_string_pretty(&schema_cache).map_err(|e| {
@@ -58,33 +69,47 @@ pub async fn run_schema_export(
5869
)))
5970
})?;
6071

61-
// Write to file
62-
std::fs::write(output_path, json).map_err(|e| {
63-
CliDiagnostic::io_error(std::io::Error::other(format!(
64-
"Failed to write output file: {e}"
65-
)))
66-
})?;
72+
// Write output
73+
if let Some(path) = output_path {
74+
std::fs::write(path, &json).map_err(|e| {
75+
CliDiagnostic::io_error(std::io::Error::other(format!(
76+
"Failed to write output file: {e}"
77+
)))
78+
})?;
6779

68-
console.log(markup! {
69-
"Schema exported to "<Emphasis>{output_path.display().to_string()}</Emphasis>
70-
});
80+
console.log(markup! {
81+
"Schema exported to "<Emphasis>{path.display().to_string()}</Emphasis>
82+
});
7183

72-
// Print summary
73-
console.log(markup! {
74-
"\nSchema summary:"
75-
});
76-
console.log(markup! {
77-
" Schemas: "{schema_cache.schemas.len().to_string()}
78-
});
79-
console.log(markup! {
80-
" Tables: "{schema_cache.tables.len().to_string()}
81-
});
82-
console.log(markup! {
83-
" Functions: "{schema_cache.functions.len().to_string()}
84-
});
85-
console.log(markup! {
86-
" Types: "{schema_cache.types.len().to_string()}
87-
});
84+
// Print summary
85+
console.log(markup! {
86+
"\nSchema summary:"
87+
});
88+
console.log(markup! {
89+
" Schemas: "{schema_cache.schemas.len().to_string()}
90+
});
91+
console.log(markup! {
92+
" Tables: "{schema_cache.tables.len().to_string()}
93+
});
94+
console.log(markup! {
95+
" Functions: "{schema_cache.functions.len().to_string()}
96+
});
97+
console.log(markup! {
98+
" Types: "{schema_cache.types.len().to_string()}
99+
});
100+
} else {
101+
// Write to stdout
102+
std::io::stdout().write_all(json.as_bytes()).map_err(|e| {
103+
CliDiagnostic::io_error(std::io::Error::other(format!(
104+
"Failed to write to stdout: {e}"
105+
)))
106+
})?;
107+
std::io::stdout().write_all(b"\n").map_err(|e| {
108+
CliDiagnostic::io_error(std::io::Error::other(format!(
109+
"Failed to write to stdout: {e}"
110+
)))
111+
})?;
112+
}
88113

89114
Ok(())
90115
}

crates/pgls_cli/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ impl<'app> CliSession<'app> {
126126
let runtime = tokio::runtime::Runtime::new().map_err(CliDiagnostic::io_error)?;
127127
runtime.block_on(commands::schema_export::run_schema_export(
128128
&connection_string,
129-
&output,
129+
output.as_deref(),
130130
))
131131
}
132132
};

crates/pgls_cli/src/metrics.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ impl CallsiteEntry {
7474
}
7575
}
7676

77-
fn into_histograms(self, name: &str) -> Vec<(Cow<str>, Histogram<u64>)> {
77+
fn into_histograms(self, name: &str) -> Vec<(Cow<'_, str>, Histogram<u64>)> {
7878
match self {
7979
CallsiteEntry::Debug { total } => vec![(Cow::Borrowed(name), total)],
8080
CallsiteEntry::Trace { total, busy, idle } => vec![

crates/pgls_console/src/markup.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -205,11 +205,11 @@ impl Write for MarkupBuf {
205205
Ok(())
206206
})?;
207207

208-
if let Some(last) = self.0.last_mut() {
209-
if last.elements == styles {
210-
last.content.push_str(content);
211-
return Ok(());
212-
}
208+
if let Some(last) = self.0.last_mut()
209+
&& last.elements == styles
210+
{
211+
last.content.push_str(content);
212+
return Ok(());
213213
}
214214

215215
self.0.push(MarkupNodeBuf {
@@ -227,11 +227,11 @@ impl Write for MarkupBuf {
227227
Ok(())
228228
})?;
229229

230-
if let Some(last) = self.0.last_mut() {
231-
if last.elements == styles {
232-
last.content.push_str(&content.to_string());
233-
return Ok(());
234-
}
230+
if let Some(last) = self.0.last_mut()
231+
&& last.elements == styles
232+
{
233+
last.content.push_str(&content.to_string());
234+
return Ok(());
235235
}
236236

237237
self.0.push(MarkupNodeBuf {

crates/pgls_diagnostics/src/display/backtrace.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,10 @@ impl NativeBacktrace {
163163
})
164164
});
165165

166-
if let Some(top_frame) = top_frame {
167-
if let Some(bottom_frames) = frames.get(top_frame + 1..) {
168-
frames = bottom_frames;
169-
}
166+
if let Some(top_frame) = top_frame
167+
&& let Some(bottom_frames) = frames.get(top_frame + 1..)
168+
{
169+
frames = bottom_frames;
170170
}
171171

172172
let bottom_frame = frames.iter().position(|frame| {
@@ -177,10 +177,10 @@ impl NativeBacktrace {
177177
})
178178
});
179179

180-
if let Some(bottom_frame) = bottom_frame {
181-
if let Some(top_frames) = frames.get(..bottom_frame + 1) {
182-
frames = top_frames;
183-
}
180+
if let Some(bottom_frame) = bottom_frame
181+
&& let Some(top_frames) = frames.get(..bottom_frame + 1)
182+
{
183+
frames = top_frames;
184184
}
185185

186186
frames

crates/pgls_workspace/src/settings.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,12 +293,12 @@ impl Settings {
293293
}
294294

295295
/// Returns linter rules.
296-
pub fn as_linter_rules(&self) -> Option<Cow<pgls_configuration::linter::Rules>> {
296+
pub fn as_linter_rules(&self) -> Option<Cow<'_, pgls_configuration::linter::Rules>> {
297297
self.linter.rules.as_ref().map(Cow::Borrowed)
298298
}
299299

300300
/// Returns splinter rules.
301-
pub fn as_splinter_rules(&self) -> Option<Cow<pgls_configuration::splinter::Rules>> {
301+
pub fn as_splinter_rules(&self) -> Option<Cow<'_, pgls_configuration::splinter::Rules>> {
302302
self.splinter.rules.as_ref().map(Cow::Borrowed)
303303
}
304304

crates/pgls_workspace/src/workspace/server.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,11 @@ impl WorkspaceServer {
124124
}
125125

126126
/// Provides a reference to the current settings
127-
fn workspaces(&self) -> WorkspaceSettingsHandle {
127+
fn workspaces(&self) -> WorkspaceSettingsHandle<'_> {
128128
WorkspaceSettingsHandle::new(&self.settings)
129129
}
130130

131-
fn workspaces_mut(&self) -> WorkspaceSettingsHandleMut {
131+
fn workspaces_mut(&self) -> WorkspaceSettingsHandleMut<'_> {
132132
WorkspaceSettingsHandleMut::new(&self.settings)
133133
}
134134

@@ -500,8 +500,10 @@ impl Workspace for WorkspaceServer {
500500
#[cfg(feature = "db")]
501501
fn invalidate_schema_cache(&self, all: bool) -> Result<(), WorkspaceError> {
502502
if all {
503-
// Clear all db-loaded schemas (keep json schema since it was explicitly set)
504-
self.schema_cache.clear_all_connections();
503+
// Clear all schemas - both db-loaded and json-loaded
504+
// DB completions always take precedence when a connection is available,
505+
// so clearing the json schema keeps behavior consistent
506+
self.schema_cache.clear_all();
505507
} else {
506508
// Only clear current connection if one exists
507509
if let Some(pool) = self.get_current_connection() {

crates/pgls_workspace/src/workspace/server/analyser.rs

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ impl<'a> AnalyserVisitorBuilder<'a> {
5050
self
5151
}
5252

53-
#[cfg(feature = "db")]
5453
#[must_use]
5554
pub(crate) fn finish(self) -> (Vec<RuleFilter<'static>>, Vec<RuleFilter<'static>>) {
5655
let mut disabled_rules = vec![];
@@ -61,6 +60,7 @@ impl<'a> AnalyserVisitorBuilder<'a> {
6160
enabled_rules.extend(linter_enabled_rules);
6261
disabled_rules.extend(linter_disabled_rules);
6362
}
63+
#[cfg(feature = "db")]
6464
if let Some(mut splinter) = self.splinter {
6565
pgls_splinter::registry::visit_registry(&mut splinter);
6666
let (splinter_enabled_rules, splinter_disabled_rules) = splinter.finish();
@@ -70,21 +70,6 @@ impl<'a> AnalyserVisitorBuilder<'a> {
7070

7171
(enabled_rules, disabled_rules)
7272
}
73-
74-
#[cfg(not(feature = "db"))]
75-
#[must_use]
76-
pub(crate) fn finish(self) -> (Vec<RuleFilter<'static>>, Vec<RuleFilter<'static>>) {
77-
let mut disabled_rules = vec![];
78-
let mut enabled_rules = vec![];
79-
if let Some(mut lint) = self.lint {
80-
pgls_analyser::visit_registry(&mut lint);
81-
let (linter_enabled_rules, linter_disabled_rules) = lint.finish();
82-
enabled_rules.extend(linter_enabled_rules);
83-
disabled_rules.extend(linter_disabled_rules);
84-
}
85-
86-
(enabled_rules, disabled_rules)
87-
}
8873
}
8974

9075
/// Type meant to register all the lint rules

docs/codegen/src/rules_index.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ fn generate_group(
7878
Ok(())
7979
}
8080

81-
fn extract_group_metadata(group: &str) -> (&str, Markup) {
81+
fn extract_group_metadata(group: &str) -> (&str, Markup<'_>) {
8282
match group {
8383
"safety" => (
8484
"Safety",

0 commit comments

Comments
 (0)