feat(format): current file in formatter args

[filipdutescu]

Add support for a current file placeholder in formatter arguments. This should make it possible to run custom formatters, such as rome (Typescript), rustfmt (Rust) and more.

The changes take inspiration from f7ecf9c and acdce30, but instead of adding a new field to the formatter struct, it works by looking for a predefined placeholder ({}), which should be replaced, if found. This should allow for better control over arguments of formatters, allowing both giving it as a standalone arg ("{}") and as part of one (ie "--src-file={}"). The placeholder, {}, is used since it is common, Rust using it frequently.

Closes: #3596
Signed-off-by: Filip Dutescu [email protected]

[the-mikedavis]

The syntax we use for this should mirror the variable expansion syntax once merged #3393

[filipdutescu]

Should I add it now or wait for that one first?

[filipdutescu]

Just to make sure, that syntax is %val{filename}, correct?

[the-mikedavis]

Currently yep that's the syntax but I think we should wait on that PR incase it's revised

[nestor-custodio]

The original issue this PR addresses is over 8 months old now and is a show-stopping autoformatting problem (for certain users under specific tooling conditions/needs) on a number of languages. This PR fixed the problem over 3 months ago and has been sitting idle waiting on an unrelated PR, just to make sure they use the same variable expansion syntax.

Am I crazy for thinking this should go out as-is and #3393 should get a note indicating that the relevant code here should be updated to match if the syntax does see a change before that PR gets final approval?

(This is not intended to sound as snippy as it probably does. Just wanted to summarize the background behind this before asking the question.)

[the-mikedavis]

We'd like to get #3393 into the next release (the one after the imminent small bugfix-like release) and I think this PR could follow #3393 shortly. I want the variable syntax to be finalized before introducing any variables so that we don't have to backtrack and make breaking changes.

This is not intended to sound as snippy as it probably does

The first paragraph does basically come across as a rant :/. If it's a show-stopper for you then you should merge the changes into a fork and use that.

[n3oney]

if anyone wants a simple patch file that does the same thing (I believe) and applies on the master version currently I wrote a simple one:

diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs
index b08370f9..bcfdafa1 100644
--- a/helix-view/src/document.rs
+++ b/helix-view/src/document.rs
@@ -735,11 +735,23 @@ pub fn format(&self) -> Option<BoxFuture<'static, Result<Transaction, FormatterE
             .and_then(|c| c.formatter.clone())
             .filter(|formatter| which::which(&formatter.command).is_ok())
         {
+            let file_path = self.path()?.to_str().unwrap_or("");
+
+            let mut args = formatter.args.clone();
+
+            for i in 0..args.len() {
+                if let Some(arg) = args.get_mut(i) {
+                    if arg.contains("{}") {
+                        *arg = arg.replace("{}", file_path);
+                    }
+                }
+            }
+
             use std::process::Stdio;
             let text = self.text().clone();
             let mut process = tokio::process::Command::new(&formatter.command);
             process
-                .args(&formatter.args)
+                .args(&args)
                 .stdin(Stdio::piped())
                 .stdout(Stdio::piped())
                 .stderr(Stdio::piped());

[filipdutescu]

Once there is an an agreed upon syntax I am happy to update the PR and fix the issue, just to give an update. But since there is currently nothing set in stone I see no use updating the PR just to further change it later on.

[n3oney]

Once there is an an agreed upon syntax I am happy to update the PR and fix the issue, just to give an update. But since there is currently nothing set in stone I see no use updating the PR just to further change it later on.

Yeah, I understand. I just wanted something quick n dirty working right now, because I'm honestly only using this as a hack to get WakaTime working with Helix. See #3477 (comment)

[IgnisDa]

if anyone wants a simple patch file that does the same thing (I believe) and applies on the master version currently I wrote a simple one:

diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs
index b08370f9..bcfdafa1 100644
--- a/helix-view/src/document.rs
+++ b/helix-view/src/document.rs
@@ -735,11 +735,23 @@ pub fn format(&self) -> Option<BoxFuture<'static, Result<Transaction, FormatterE
             .and_then(|c| c.formatter.clone())
             .filter(|formatter| which::which(&formatter.command).is_ok())
         {
+            let file_path = self.path()?.to_str().unwrap_or("");
+
+            let mut args = formatter.args.clone();
+
+            for i in 0..args.len() {
+                if let Some(arg) = args.get_mut(i) {
+                    if arg.contains("{}") {
+                        *arg = arg.replace("{}", file_path);
+                    }
+                }
+            }
+
             use std::process::Stdio;
             let text = self.text().clone();
             let mut process = tokio::process::Command::new(&formatter.command);
             process
-                .args(&formatter.args)
+                .args(&args)
                 .stdin(Stdio::piped())
                 .stdout(Stdio::piped())
                 .stderr(Stdio::piped());

Could you tell me how to apply these changes to the current main?

[tynanbe]

I'm using this as a workaround.

formatter = { command = "sh", args = ["-c", """
  file=$(
    ps -p "${PPID}" -o "command=" \
      | awk '{ print $NF }'
  )
  exec prettier \
    --stdin-filepath="${file}" \
    --config-precedence=file-override
"""] }