Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run formatter from Document directory #12315

Conversation

robgonnella
Copy link
Contributor

@robgonnella robgonnella commented Dec 22, 2024

Addresses issue #10187

@robgonnella robgonnella force-pushed the issue-10187-use-document-dir-when-formatting branch from 739cda7 to 75c8980 Compare December 22, 2024 05:09
@robgonnella robgonnella changed the title Run formatter from Document directory (#10187) Run formatter from Document directory Dec 22, 2024
@robgonnella
Copy link
Contributor Author

This is an attempt to solve an issue where the external formatter is run from the directory in which helix was launched rather than from the working directory of the document / buffer being updated. In mono-repos this results in a failure to detect formatter configs in subdirectories.

I'm new to Rust and this project so apologies if my approach is way off here. Any feedback is greatly appreciated!

@robgonnella robgonnella force-pushed the issue-10187-use-document-dir-when-formatting branch 2 times, most recently from 2c8fe9f to 93d1514 Compare December 27, 2024 16:12
@robgonnella
Copy link
Contributor Author

@the-mikedavis gentle ping. I know that there are a lot PRs, discussions, and work to attend to so not trying to push here, I just wasn't sure of the protocol for requesting a review and didn't see anything mentioned in the Contributing doc. Thank you for all your awesome work on Helix! It's now my daily driver!

helix-view/src/document.rs Outdated Show resolved Hide resolved
@robgonnella robgonnella force-pushed the issue-10187-use-document-dir-when-formatting branch from 93d1514 to d8ce8cd Compare January 7, 2025 23:15
let cwd = std::env::current_dir().unwrap_or(PathBuf::from("."));
let doc_path = self.path.clone().unwrap_or_else(|| cwd.join(self.display_name().to_string()));
let doc_dir = doc_path.parent().unwrap_or(cwd.as_path());

let mut process = tokio::process::Command::new(&fmt_cmd);
Copy link
Member

Choose a reason for hiding this comment

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

We can avoid setting current_dir when the document doesn't have a path (i.e. is a scratch buffer) so that the process is started from the CWD instead.

if let Some(doc_dir) = self.path.as_ref().and_then(|path| path.parent()) {
    process.current_dir(doc_dir);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh nice thank you! Tested locally and this works well too. Updated PR

I thought since process.current_dir returns &mut Command that maybe it needed to be chained.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah it's a cool idiom that you can write an interface as fn foo(&mut self) -> &mut Self and use it to chain calls or to make an update in place

Fixes an issue where the external formatter is run from the
directory in which helix was launched rather than from the
working directory of the document / buffer being updated.
In mono-repos this results in a failure to detect formatter
configs in subdirectories.

Addresses issue helix-editor#10187
@robgonnella robgonnella force-pushed the issue-10187-use-document-dir-when-formatting branch from d8ce8cd to 0687ac1 Compare January 8, 2025 16:57
@the-mikedavis the-mikedavis merged commit a83c23b into helix-editor:master Jan 8, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants