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.

Sign up for GitHub

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

Enable DartFmt Options with g:dartfmt_options #108

Merged
merged 5 commits into from
Jun 25, 2020
Merged

Enable DartFmt Options with g:dartfmt_options #108

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
da61cf0
allow dartfmt options by g:dartfmt_options
May 8, 2020
a0ec2d8
added documentations for g:dartfmt_options
May 20, 2020
4ab035a
added suggested changes
Jun 8, 2020
8ebd2c5
Switch the option to a list
natebosch Jun 8, 2020
8962b39
Remove extra list wrapping for flutter case
natebosch Jun 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ Enable Dart style guide syntax (like 2-space indentation) with

Enable DartFmt execution on buffer save with `let g:dart_format_on_save = 1`

Configure DartFmt Options with `let g:dartfmt_options`, for example, change
line limit to 100 (default 80) and enable auto syntax fixes with
Copy link
Member

Choose a reason for hiding this comment

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

If we merge this we wouldn't want to encourage usage of setting the line limit. Idiomatic usage for dartfmt is to accept the default. We might even want to leave it out of the README entirely since most users shouldn't care about it, and add a note to the doc instead. cc @cbracken - what are you thoughts on how much to put in the README?

I can see the value in supporting this for the --fix arguments, but I'm on the fence about it. I think we typically expect those to be used infrequently, rather than on every run...

Copy link
Contributor Author

@theniceboy theniceboy May 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Personally, I find the 80 line limit annoying, and it looks like there are other people who have the same issue (check out this issue from the dartfmt repo).

I did a search and found that ~35% of our organization's Dart projects (maintained by various independent teams) use a custom dartfmt line length of either 100 or 120 as opposed to the default 80.

Maybe we should explain it more and recommend the default 80 line limit.

For --fix, I find it helpful but it does increase the :wq lag by a little bit. I guess we want to make that clear.

@natebosch

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should explain it more and recommend the default 80 line limit.

I think it's fine for folks to use dartfmt -h to discover the options, we don't need to call them out specifically here. We can mention --fix as the motivating use case in our docs.

I do think this should be reserved for the help docs over the README.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense.

`let g:dartfmt_options = "-l 100 --fix"`
(see more options with `dartfmt -h`)

## FAQ

### Why doesn't the plugin indent identically to `dartfmt`?
Expand Down
4 changes: 2 additions & 2 deletions autoload/dart.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ function! dart#fmt(q_args) abort
endfunction

function! s:FindDartFmt() abort
if executable('dartfmt') | return 'dartfmt' | endif
if executable('dartfmt') | return 'dartfmt '.g:dartfmt_options | endif
if executable('flutter')
let l:flutter_cmd = resolve(exepath('flutter'))
let l:bin = fnamemodify(l:flutter_cmd, ':h')
let l:dartfmt = l:bin.'/cache/dart-sdk/bin/dartfmt'
let l:dartfmt = l:bin.'/cache/dart-sdk/bin/dartfmt '.g:dartfmt_options
if executable(l:dartfmt) | return l:dartfmt | endif
endif
call s:error('Cannot find a `dartfmt` command')
Expand Down