google / vim-codefmt

Vim plugin for syntax-aware code formatting
Apache License 2.0
1.11k stars 114 forks source link

dartfmt is deprecated #190

Closed chengen closed 1 year ago

chengen commented 2 years ago

Describe the bug The dart format command replaces dartfmt. Dart SDK version: 2.16.1 already removed dartfmt command line tool. I try to use alias as a work around, but not working. alias dartfmt='dart format -o show'

thanks!

malcolmr commented 2 years ago

That's... annoyingly quick, as deprecations go.

I think we might want to basically replicate the functionality from https://github.com/dart-lang/dart-vim-plugin/pull/125, which checks whether dart exists, then does a version match on the output of dart --version, and uses dart format if the version is >= 2.14.

dbarnett commented 2 years ago

Shell alias is completely ignored in vim syscalls IIRC. Did you try setting dartfmt_executable instead? Something like Glaive codefmt dartfmt_executable='dart format -o show'? I can't remember if we set that one up to split spaces properly yet, so that might just give an error about the spaces if not.

chengen commented 2 years ago

That's... annoyingly quick, as deprecations go.

I think we might want to basically replicate the functionality from dart-lang/dart-vim-plugin#125, which checks whether dart exists, then does a version match on the output of dart --version, and uses dart format if the version is >= 2.14.

Thank you malcolmr, that would be great if codefmt solve this by check dart version.

chengen commented 2 years ago

I find a simple work around here, Just put a small excutable shell script name 'dartfmt' under one of your $PATH directory, for example: /usr/local/bin/ , where 'which' command can find it.

#!/bin/sh
dart format -o show

Make sure the which command can find it! At the beginning I put it under my personal bin directory(which is added to the $PATH), it didn't work. Then I put it under /usr/local/bin, it works perfect.

dbarnett commented 2 years ago

Yes, that should work if the dartfmt_executable override doesn't. We should still make sure that override works and follow up about the version check.

I'd suggest to minimize overhead for the new case by always attempting dart format first, then falling back to a version check and dartfmt for whatever error cases suggest the new version is missing.

chengen commented 2 years ago

@dbarnett This doesn't work because of the spaces.

:Glaive codefmt dartfmt_executable='dart format -o show'

By adding dartfmt_options as below, it works:

--- a/autoload/codefmt/dartfmt.vim
+++ b/autoload/codefmt/dartfmt.vim
@@ -38,7 +38,7 @@ function! codefmt#dartfmt#GetFormatter() abort
   " @flag(dartfmt_executable}, only targetting the range from {startline} to
   " {endline}
   function l:formatter.FormatRange(startline, endline) abort
-    let l:cmd = [ s:plugin.Flag('dartfmt_executable') ]
+    let l:cmd = [ s:plugin.Flag('dartfmt_executable') ] + s:plugin.Flag('dartfmt_options')
     try
       " dartfmt does not support range formatting yet:
       " https://github.com/dart-lang/dart_style/issues/92
diff --git a/instant/flags.vim b/instant/flags.vim
index 8694c31..5a7a744 100644
--- a/instant/flags.vim
+++ b/instant/flags.vim
@@ -74,7 +74,11 @@ call s:plugin.Flag('gofmt_executable', 'gofmt')

 ""
 " The path to the dartfmt executable.
-call s:plugin.Flag('dartfmt_executable', 'dartfmt')
+call s:plugin.Flag('dartfmt_executable', 'dart')
+
+""
+" The options to feed new dartfmt
+call s:plugin.Flag('dartfmt_options', ['format', '-o', 'show'])
dbarnett commented 2 years ago

K, then it needs a fix like 293c2088fc to support spaces. Thanks for checking. In the meantime, your shell script workaround is the best bet.

malcolmr commented 1 year ago

Note that this was mostly done in #224, though I see we actually have a bug: we switched dartfmt_executable to optionally be a list, but https://github.com/google/vim-codefmt/blob/dbbbca4/autoload/codefmt/dartfmt.vim#L29 still assumes that it's a string.