Skip to content

Commit 27b6e7f

Browse files
authored
refactor: drop change.rs <3 (#447)
![patrick-zen png](https://github.com/user-attachments/assets/bc491335-4f23-47d0-ace6-2c408eb1e93e) as per PR title. notable changes: - drop `change.rs` in favour of a simple `update_document` api that replaces the content - simplify `StatementId` to just be a simple wrapper around the actual statement string. The content is wrapped in an `Arc<str>` to make cloning cheap. turns out string interning only makes sense when we have a lot of duplicate strings. `Arc<str>` is much more efficient and simpler in our case. - Remove `document.rs` and merge it into `parsed_document` to then rename it to `Document` - replaces the `strum` usage to get the available command because it didn't feel right to make an empty string the default for a StatementId. replaced it with a simple manual implementation. I ran a few benchmarks and the statement splitter performance seems to be good enough to run it on every keystroke on the entire file. ```sh large statement with length 863000 time: [23.180 ms 23.278 ms 23.386 ms] small statement with length 19000 time: [886.69 µs 890.91 µs 896.35 µs] ``` ToDo - [ ] add some tests - [x] cleanup
1 parent 155272a commit 27b6e7f

File tree

17 files changed

+690
-2545
lines changed

17 files changed

+690
-2545
lines changed

.claude/settings.local.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
"Bash(cargo test:*)",
77
"Bash(cargo run:*)",
88
"Bash(cargo check:*)",
9-
"Bash(cargo fmt:*)"
9+
"Bash(cargo fmt:*)",
10+
"Bash(cargo doc:*)"
1011
],
1112
"deny": []
1213
}

Cargo.lock

Lines changed: 120 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/pgt_cli/src/execute/process_file/workspace_file.rs

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@ use crate::execute::diagnostics::{ResultExt, ResultIoExt};
22
use crate::execute::process_file::SharedTraversalOptions;
33
use pgt_diagnostics::{Error, category};
44
use pgt_fs::{File, OpenOptions, PgTPath};
5-
use pgt_workspace::workspace::{ChangeParams, FileGuard, OpenFileParams};
5+
use pgt_workspace::workspace::{FileGuard, OpenFileParams};
66
use pgt_workspace::{Workspace, WorkspaceError};
77
use std::path::{Path, PathBuf};
88

99
/// Small wrapper that holds information and operations around the current processed file
1010
pub(crate) struct WorkspaceFile<'ctx, 'app> {
1111
guard: FileGuard<'app, dyn Workspace + 'ctx>,
12+
#[allow(dead_code)]
1213
file: Box<dyn File>,
1314
pub(crate) path: PathBuf,
1415
}
@@ -57,19 +58,4 @@ impl<'ctx, 'app> WorkspaceFile<'ctx, 'app> {
5758
pub(crate) fn input(&self) -> Result<String, WorkspaceError> {
5859
self.guard().get_file_content()
5960
}
60-
61-
/// It updates the workspace file with `new_content`
62-
#[allow(dead_code)]
63-
pub(crate) fn update_file(&mut self, new_content: impl Into<String>) -> Result<(), Error> {
64-
let new_content = new_content.into();
65-
66-
self.file
67-
.set_content(new_content.as_bytes())
68-
.with_file_path(self.path.display().to_string())?;
69-
self.guard.change_file(
70-
self.file.file_version(),
71-
vec![ChangeParams::overwrite(new_content)],
72-
)?;
73-
Ok(())
74-
}
7561
}

crates/pgt_lsp/src/capabilities.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::adapters::{PositionEncoding, WideEncoding, negotiated_encoding};
2+
use crate::handlers::code_actions::command_id;
23
use pgt_workspace::features::code_actions::CommandActionCategory;
34
use strum::IntoEnumIterator;
45
use tower_lsp::lsp_types::{
@@ -7,8 +8,6 @@ use tower_lsp::lsp_types::{
78
TextDocumentSyncOptions, TextDocumentSyncSaveOptions, WorkDoneProgressOptions,
89
};
910

10-
use crate::handlers::code_actions::command_id;
11-
1211
/// The capabilities to send from server as part of [`InitializeResult`]
1312
///
1413
/// [`InitializeResult`]: lspower::lsp::InitializeResult
@@ -54,7 +53,6 @@ pub(crate) fn server_capabilities(capabilities: &ClientCapabilities) -> ServerCa
5453
commands: CommandActionCategory::iter()
5554
.map(|c| command_id(&c))
5655
.collect::<Vec<String>>(),
57-
5856
..Default::default()
5957
}),
6058
document_formatting_provider: None,

crates/pgt_lsp/src/handlers/text_document.rs

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
use crate::adapters::from_lsp;
21
use crate::{
32
diagnostics::LspError, documents::Document, session::Session, utils::apply_document_changes,
43
};
54
use anyhow::Result;
65
use pgt_workspace::workspace::{
7-
ChangeFileParams, ChangeParams, CloseFileParams, GetFileContentParams, OpenFileParams,
6+
ChangeFileParams, CloseFileParams, GetFileContentParams, OpenFileParams,
87
};
98
use tower_lsp::lsp_types;
109
use tracing::error;
@@ -48,40 +47,28 @@ pub(crate) async fn did_change(
4847

4948
let pgt_path = session.file_path(&url)?;
5049

51-
let old_doc = session.document(&url)?;
5250
let old_text = session.workspace.get_file_content(GetFileContentParams {
5351
path: pgt_path.clone(),
5452
})?;
55-
56-
let start = params
57-
.content_changes
58-
.iter()
59-
.rev()
60-
.position(|change| change.range.is_none())
61-
.map_or(0, |idx| params.content_changes.len() - idx - 1);
53+
tracing::trace!("old document: {:?}", old_text);
54+
tracing::trace!("content changes: {:?}", params.content_changes);
6255

6356
let text = apply_document_changes(
6457
session.position_encoding(),
6558
old_text,
66-
&params.content_changes[start..],
59+
&params.content_changes,
6760
);
6861

62+
tracing::trace!("new document: {:?}", text);
63+
64+
session.insert_document(url.clone(), Document::new(version, &text));
65+
6966
session.workspace.change_file(ChangeFileParams {
7067
path: pgt_path,
7168
version,
72-
changes: params.content_changes[start..]
73-
.iter()
74-
.map(|c| ChangeParams {
75-
range: c.range.and_then(|r| {
76-
from_lsp::text_range(&old_doc.line_index, r, session.position_encoding()).ok()
77-
}),
78-
text: c.text.clone(),
79-
})
80-
.collect(),
69+
content: text,
8170
})?;
8271

83-
session.insert_document(url.clone(), Document::new(version, &text));
84-
8572
if let Err(err) = session.update_diagnostics(url).await {
8673
error!("Failed to update diagnostics: {}", err);
8774
}

crates/pgt_statement_splitter/Cargo.toml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,9 @@ pgt_text_size.workspace = true
1919
regex.workspace = true
2020

2121
[dev-dependencies]
22-
ntest = "0.9.3"
22+
criterion = "0.3"
23+
ntest = "0.9.3"
24+
25+
[[bench]]
26+
harness = false
27+
name = "splitter"

0 commit comments

Comments
 (0)