Skip to content

dartfmt is deprecated #190

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

Closed
chengen opened this issue Feb 23, 2022 · 8 comments · Fixed by #225
Closed

dartfmt is deprecated #190

chengen opened this issue Feb 23, 2022 · 8 comments · Fixed by #225
Labels

Comments

@chengen
Copy link

chengen commented Feb 23, 2022

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 chengen added the bug label Feb 23, 2022
@malcolmr
Copy link
Member

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.

@dbarnett
Copy link
Contributor

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
Copy link
Author

chengen commented Feb 26, 2022

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
Copy link
Author

chengen commented Feb 26, 2022

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
Copy link
Contributor

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
Copy link
Author

chengen commented Feb 27, 2022

@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
Copy link
Contributor

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

@malcolmr
Copy link
Member

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 added a commit to malcolmr/vim-codefmt that referenced this issue Aug 15, 2023
d927656 changed the dartfmt_executable flag from a string to a list, but
didn't change the use inside IsAvailable(), leading to:

Failed to evaluate whether formatter dartfmt is available: Vim(return)
:E730: Using a List as a String

The change uses the ResolveFlagToArray() helper in both IsAvailable()
and FormatRange() (which incidentally also allows defining the flag
using a Function), and updates the documentation for all flags using
ResolveFlagToArray() to use the same phrasing.

Fixes google#190.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants