From 32dfc1647b380b39fdecab0f7c3cfb9278659297 Mon Sep 17 00:00:00 2001 From: rolo Date: Sun, 20 Jul 2025 15:51:20 -0700 Subject: [PATCH] refactor(document): change `path` output to `&Path` --- helix-core/src/uri.rs | 6 ++++++ helix-lsp/src/client.rs | 2 +- helix-lsp/src/lib.rs | 8 ++++---- helix-term/src/commands.rs | 6 +++--- helix-term/src/commands/dap.rs | 23 ++++++++++------------- helix-term/src/commands/syntax.rs | 8 +++----- helix-term/src/commands/typed.rs | 15 +++++++-------- helix-term/src/ui/editor.rs | 5 ++--- helix-term/src/ui/statusline.rs | 8 ++++---- helix-term/tests/test/commands/write.rs | 6 +++--- helix-view/src/document.rs | 12 +++++++++--- helix-view/src/editor.rs | 8 ++++---- helix-view/src/gutter.rs | 3 ++- 13 files changed, 58 insertions(+), 52 deletions(-) diff --git a/helix-core/src/uri.rs b/helix-core/src/uri.rs index cbe0fadda..a8f9f307f 100644 --- a/helix-core/src/uri.rs +++ b/helix-core/src/uri.rs @@ -37,6 +37,12 @@ impl From for Uri { } } +impl From<&Path> for Uri { + fn from(path: &Path) -> Self { + Self::File(path.into()) + } +} + impl fmt::Display for Uri { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs index afb3b3a56..7472da9bb 100644 --- a/helix-lsp/src/client.rs +++ b/helix-lsp/src/client.rs @@ -69,7 +69,7 @@ impl Client { self: &Arc, root_markers: &[String], manual_roots: &[PathBuf], - doc_path: Option<&std::path::PathBuf>, + doc_path: Option<&std::path::Path>, may_support_workspace: bool, ) -> bool { let (workspace, workspace_is_cwd) = find_workspace(); diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index 567e8a702..841aa3c75 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -591,7 +591,7 @@ impl Registry { &mut self, name: String, ls_config: &LanguageConfiguration, - doc_path: Option<&std::path::PathBuf>, + doc_path: Option<&std::path::Path>, root_dirs: &[PathBuf], enable_snippets: bool, ) -> Result, StartupError> { @@ -624,7 +624,7 @@ impl Registry { &mut self, name: &str, language_config: &LanguageConfiguration, - doc_path: Option<&std::path::PathBuf>, + doc_path: Option<&std::path::Path>, root_dirs: &[PathBuf], enable_snippets: bool, ) -> Option>> { @@ -679,7 +679,7 @@ impl Registry { pub fn get<'a>( &'a mut self, language_config: &'a LanguageConfiguration, - doc_path: Option<&'a std::path::PathBuf>, + doc_path: Option<&'a std::path::Path>, root_dirs: &'a [PathBuf], enable_snippets: bool, ) -> impl Iterator>)> + 'a { @@ -867,7 +867,7 @@ fn start_client( name: String, config: &LanguageConfiguration, ls_config: &LanguageServerConfiguration, - doc_path: Option<&std::path::PathBuf>, + doc_path: Option<&std::path::Path>, root_dirs: &[PathBuf], enable_snippets: bool, ) -> Result { diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index a3417ea1b..43529a732 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2522,7 +2522,7 @@ fn global_search(cx: &mut Context) { let documents: Vec<_> = editor .documents() - .map(|doc| (doc.path().cloned(), doc.text().to_owned())) + .map(|doc| (doc.pathbuf(), doc.text().to_owned())) .collect(); let matcher = match RegexMatcherBuilder::new() @@ -3157,7 +3157,7 @@ fn buffer_picker(cx: &mut Context) { let new_meta = |doc: &Document| BufferMeta { id: doc.id(), - path: doc.path().cloned(), + path: doc.pathbuf(), is_modified: doc.is_modified(), is_current: doc.id() == current, focused_at: doc.focused_at, @@ -3239,7 +3239,7 @@ fn jumplist_picker(cx: &mut Context) { JumpMeta { id: doc_id, - path: doc.and_then(|d| d.path().cloned()), + path: doc.and_then(|doc| doc.pathbuf()), selection, text, is_current: view.doc == doc_id, diff --git a/helix-term/src/commands/dap.rs b/helix-term/src/commands/dap.rs index f6f11d126..d3c6e6b01 100644 --- a/helix-term/src/commands/dap.rs +++ b/helix-term/src/commands/dap.rs @@ -376,14 +376,13 @@ fn debug_parameter_prompt( pub fn dap_toggle_breakpoint(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let path = match doc.path() { - Some(path) => path.clone(), - None => { - cx.editor - .set_error("Can't set breakpoint: document has no path"); - return; - } + + let Some(path) = doc.pathbuf() else { + cx.editor + .set_error("Can't set breakpoint: document has no path"); + return; }; + let text = doc.text().slice(..); let line = doc.selection(view.id).primary().cursor_line(text); dap_toggle_breakpoint_impl(cx, path, line); @@ -623,9 +622,8 @@ pub fn dap_disable_exceptions(cx: &mut Context) { // TODO: both edit condition and edit log need to be stable: we might get new breakpoints from the debugger which can change offsets pub fn dap_edit_condition(cx: &mut Context) { if let Some((pos, breakpoint)) = get_breakpoint_at_current_line(cx.editor) { - let path = match doc!(cx.editor).path() { - Some(path) => path.clone(), - None => return, + let Some(path) = doc!(cx.editor).pathbuf() else { + return; }; let callback = Box::pin(async move { let call: Callback = Callback::EditorCompositor(Box::new(move |editor, compositor| { @@ -665,9 +663,8 @@ pub fn dap_edit_condition(cx: &mut Context) { pub fn dap_edit_log(cx: &mut Context) { if let Some((pos, breakpoint)) = get_breakpoint_at_current_line(cx.editor) { - let path = match doc!(cx.editor).path() { - Some(path) => path.clone(), - None => return, + let Some(path) = doc!(cx.editor).pathbuf() else { + return; }; let callback = Box::pin(async move { let call: Callback = Callback::EditorCompositor(Box::new(move |editor, compositor| { diff --git a/helix-term/src/commands/syntax.rs b/helix-term/src/commands/syntax.rs index fec222ce8..7a3d75f8e 100644 --- a/helix-term/src/commands/syntax.rs +++ b/helix-term/src/commands/syntax.rs @@ -315,11 +315,9 @@ pub fn syntax_workspace_symbol_picker(cx: &mut Context) { let pattern = Arc::new(pattern); let injector = injector.clone(); let loader = editor.syn_loader.load(); - let documents: HashSet<_> = editor - .documents() - .filter_map(Document::path) - .cloned() - .collect(); + + let documents: HashSet<_> = editor.documents().filter_map(Document::pathbuf).collect(); + async move { let searcher = state.searcher_builder.build(); state.walk_builder.build_parallel().run(|| { diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 82cad8386..59476f2bd 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -181,7 +181,7 @@ fn buffer_gather_paths_impl(editor: &mut Editor, args: Args) -> Vec for arg in args { let doc_id = editor.documents().find_map(|doc| { let arg_path = Some(Path::new(arg.as_ref())); - if doc.path().map(|p| p.as_path()) == arg_path || doc.relative_path() == arg_path { + if doc.path() == arg_path || doc.relative_path() == arg_path { Some(doc.id()) } else { None @@ -1397,11 +1397,11 @@ fn reload(cx: &mut compositor::Context, _args: Args, event: PromptEvent) -> anyh doc.reload(view, &cx.editor.diff_providers).map(|_| { view.ensure_cursor_in_view(doc, scrolloff); })?; - if let Some(path) = doc.path() { + if let Some(path) = doc.pathbuf() { cx.editor .language_servers .file_event_handler - .file_changed(path.clone()); + .file_changed(path); } Ok(()) } @@ -1443,11 +1443,11 @@ fn reload_all(cx: &mut compositor::Context, _args: Args, event: PromptEvent) -> continue; } - if let Some(path) = doc.path() { + if let Some(path) = doc.pathbuf() { cx.editor .language_servers .file_event_handler - .file_changed(path.clone()); + .file_changed(path); } for view_id in view_ids { @@ -2525,9 +2525,8 @@ fn move_buffer(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> let doc = doc!(cx.editor); let old_path = doc - .path() - .context("Scratch buffer cannot be moved. Use :write instead")? - .clone(); + .pathbuf() + .context("Scratch buffer cannot be moved. Use :write instead")?; let new_path: PathBuf = args.first().unwrap().into(); // if new_path is a directory, append the original file name diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 9343d55d4..16a497437 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -1160,9 +1160,8 @@ impl EditorView { let (view, doc) = current!(cxt.editor); - let path = match doc.path() { - Some(path) => path.clone(), - None => return EventResult::Ignored(None), + let Some(path) = doc.pathbuf() else { + return EventResult::Ignored(None); }; if let Some(char_idx) = diff --git a/helix-term/src/ui/statusline.rs b/helix-term/src/ui/statusline.rs index ea3d27bd6..4461698ec 100644 --- a/helix-term/src/ui/statusline.rs +++ b/helix-term/src/ui/statusline.rs @@ -466,11 +466,11 @@ where F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy, { let title = { - let path = context.doc.path(); - let path = path + let path = context + .doc + .path() .as_ref() - .map(|p| p.to_string_lossy()) - .unwrap_or_else(|| SCRATCH_BUFFER_NAME.into()); + .map_or_else(|| SCRATCH_BUFFER_NAME.into(), |p| p.to_string_lossy()); format!(" {} ", path) }; diff --git a/helix-term/tests/test/commands/write.rs b/helix-term/tests/test/commands/write.rs index 0cf09e1ea..8102bcf5e 100644 --- a/helix-term/tests/test/commands/write.rs +++ b/helix-term/tests/test/commands/write.rs @@ -24,7 +24,7 @@ async fn test_write_quit_fail() -> anyhow::Result<()> { assert_eq!(1, docs.len()); let doc = docs.pop().unwrap(); - assert_eq!(Some(&path::normalize(file.path())), doc.path()); + assert_eq!(Some(path::normalize(file.path())), doc.pathbuf()); assert_eq!(&Severity::Error, app.editor.get_status().unwrap().1); }), false, @@ -265,7 +265,7 @@ async fn test_write_scratch_to_new_path() -> anyhow::Result<()> { assert_eq!(1, docs.len()); let doc = docs.pop().unwrap(); - assert_eq!(Some(&path::normalize(file.path())), doc.path()); + assert_eq!(Some(path::normalize(file.path())), doc.pathbuf()); }), false, ) @@ -651,7 +651,7 @@ async fn test_symlink_write_fail() -> anyhow::Result<()> { assert_eq!(1, docs.len()); let doc = docs.pop().unwrap(); - assert_eq!(Some(&path::normalize(&symlink_path)), doc.path()); + assert_eq!(Some(path::normalize(&symlink_path)), doc.pathbuf()); assert_eq!(&Severity::Error, app.editor.get_status().unwrap().1); }), false, diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 04b7703c5..c8404029b 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1934,8 +1934,14 @@ impl Document { #[inline] /// File path on disk. - pub fn path(&self) -> Option<&PathBuf> { - self.path.as_ref() + pub fn path(&self) -> Option<&Path> { + self.path.as_deref() + } + + #[inline] + /// Owned file path on disk. + pub fn pathbuf(&self) -> Option { + self.path.clone() } /// File path as a URL. @@ -1944,7 +1950,7 @@ impl Document { } pub fn uri(&self) -> Option { - Some(self.path()?.clone().into()) + Some(self.path()?.into()) } #[inline] diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 57e130881..748c73b73 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1540,14 +1540,14 @@ impl Editor { let Some(doc_url) = doc.url() else { return; }; - let (lang, path) = (doc.language.clone(), doc.path().cloned()); + let (lang, path) = (doc.language.clone(), doc.path()); let config = doc.config.load(); let root_dirs = &config.workspace_lsp_roots; // store only successfully started language servers let language_servers = lang.as_ref().map_or_else(HashMap::default, |language| { self.language_servers - .get(language, path.as_ref(), root_dirs, config.lsp.snippets) + .get(language, path, root_dirs, config.lsp.snippets) .filter_map(|(lang, client)| match client { Ok(client) => Some((lang, client)), Err(err) => { @@ -2061,12 +2061,12 @@ impl Editor { pub fn document_by_path>(&self, path: P) -> Option<&Document> { self.documents() - .find(|doc| doc.path().map(|p| p == path.as_ref()).unwrap_or(false)) + .find(|doc| doc.path().is_some_and(|p| p == path.as_ref())) } pub fn document_by_path_mut>(&mut self, path: P) -> Option<&mut Document> { self.documents_mut() - .find(|doc| doc.path().map(|p| p == path.as_ref()).unwrap_or(false)) + .find(|doc| doc.path().is_some_and(|p| p == path.as_ref())) } /// Returns all supported diagnostics for the document diff --git a/helix-view/src/gutter.rs b/helix-view/src/gutter.rs index c2cbc0da5..13bb1fac5 100644 --- a/helix-view/src/gutter.rs +++ b/helix-view/src/gutter.rs @@ -284,8 +284,9 @@ fn execution_pause_indicator<'doc>( frame .source .as_ref() - .and_then(|source| source.path.as_ref()) + .and_then(|source| source.path.as_deref()) }); + let should_display_for_current_doc = doc.path().is_some() && frame_source_path.unwrap_or(None) == doc.path();