vim-codefmt icon indicating copy to clipboard operation
vim-codefmt copied to clipboard

dartfmt is deprecated

Open chengen opened this issue 3 years ago • 7 comments

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!

chengen avatar Feb 23 '22 21:02 chengen

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.

malcolmr avatar Feb 23 '22 21:02 malcolmr

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.

dbarnett avatar Feb 25 '22 04:02 dbarnett

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 avatar Feb 26 '22 17:02 chengen

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.

chengen avatar Feb 26 '22 17:02 chengen

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.

dbarnett avatar Feb 26 '22 19:02 dbarnett

@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'])

chengen avatar Feb 27 '22 04:02 chengen

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.

dbarnett avatar Feb 27 '22 04:02 dbarnett

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.

malcolmr avatar Aug 15 '23 13:08 malcolmr