From d579038fad1d14ddaa900fafea67ed4d5041c4a4 Mon Sep 17 00:00:00 2001 From: David Barnett Date: Tue, 7 Jan 2020 20:18:17 -0800 Subject: [PATCH 1/5] Implement common helper functions for common formatting operations --- autoload/codefmt/autopep8.vim | 27 ++++-------- autoload/codefmt/buildifier.vim | 6 +-- autoload/codefmt/dartfmt.vim | 21 +++------- autoload/codefmt/formatterhelpers.vim | 59 +++++++++++++++++++++++++++ autoload/codefmt/gofmt.vim | 16 ++------ autoload/codefmt/googlejava.vim | 5 +-- autoload/codefmt/jsbeautify.vim | 15 ++----- autoload/codefmt/rustfmt.vim | 7 +--- autoload/codefmt/shfmt.vim | 20 +++------ autoload/codefmt/zprint.vim | 30 +++++--------- doc/codefmt.txt | 19 +++++++++ 11 files changed, 123 insertions(+), 102 deletions(-) create mode 100644 autoload/codefmt/formatterhelpers.vim diff --git a/autoload/codefmt/autopep8.vim b/autoload/codefmt/autopep8.vim index 53d5a66..40c5db1 100644 --- a/autoload/codefmt/autopep8.vim +++ b/autoload/codefmt/autopep8.vim @@ -67,29 +67,18 @@ function! codefmt#autopep8#GetFormatter() abort call maktaba#ensure#IsNumber(a:startline) call maktaba#ensure#IsNumber(a:endline) - let l:lines = getline(1, line('$')) if s:autopep8_supports_range - let l:cmd = [l:executable, '--range', ''.a:startline, ''.a:endline, '-'] - let l:input = join(l:lines, "\n") + call codefmt#formatterhelpers#Format(maktaba#syscall#Create([ + \ l:executable, + \ '--range', string(a:startline), string(a:endline), + \ '-'])) else - let l:cmd = [l:executable, '-'] - " Hack range formatting by formatting range individually, ignoring context. - let l:input = join(l:lines[a:startline - 1 : a:endline - 1], "\n") + call codefmt#formatterhelpers#AttemptFakeRangeFormatting( + \ a:startline, + \ a:endline, + \ maktaba#syscall#Create([l:executable, '-'])) endif - - let l:result = maktaba#syscall#Create(l:cmd).WithStdin(l:input).Call() - let l:formatted = split(l:result.stdout, "\n") - - if s:autopep8_supports_range - let l:full_formatted = l:formatted - else - " Special case empty slice: neither l:lines[:0] nor l:lines[:-1] is right. - let l:before = a:startline > 1 ? l:lines[ : a:startline - 2] : [] - let l:full_formatted = l:before + l:formatted + l:lines[a:endline :] - endif - - call maktaba#buffer#Overwrite(1, line('$'), l:full_formatted) endfunction return l:formatter diff --git a/autoload/codefmt/buildifier.vim b/autoload/codefmt/buildifier.vim index 822759a..ff964f3 100644 --- a/autoload/codefmt/buildifier.vim +++ b/autoload/codefmt/buildifier.vim @@ -47,9 +47,9 @@ function! codefmt#buildifier#GetFormatter() abort let l:input = join(getline(1, line('$')), "\n") try - let l:result = maktaba#syscall#Create(l:cmd).WithStdin(l:input).Call() - let l:formatted = split(l:result.stdout, "\n") - call maktaba#buffer#Overwrite(1, line('$'), l:formatted) + " NOTE: Ignores any line ranges given and formats entire buffer. + " buildifier does not support range formatting. + call codefmt#formatterhelpers#Format(l:cmd) catch " Parse all the errors and stick them in the quickfix list. let l:errors = [] diff --git a/autoload/codefmt/dartfmt.vim b/autoload/codefmt/dartfmt.vim index ba965c7..f0ed5c6 100644 --- a/autoload/codefmt/dartfmt.vim +++ b/autoload/codefmt/dartfmt.vim @@ -38,22 +38,12 @@ function! codefmt#dartfmt#GetFormatter() abort " @flag(dartfmt_executable}, only targetting the range from {startline} to " {endline} function l:formatter.FormatRange(startline, endline) abort - " Hack range formatting by formatting range individually, ignoring context. let l:cmd = [ s:plugin.Flag('dartfmt_executable') ] - " TODO When https://github.com/dart-lang/dart_style/issues/92 is implemented - " use those options. - call maktaba#ensure#IsNumber(a:startline) - call maktaba#ensure#IsNumber(a:endline) - let l:lines = getline(1, line('$')) - let l:input = join(l:lines[a:startline - 1 : a:endline - 1], "\n") try - let l:result = maktaba#syscall#Create(l:cmd).WithStdin(l:input).Call() - let l:formatted = split(l:result.stdout, "\n") - " Special case empty slice: neither l:lines[:0] nor l:lines[:-1] is right. - let l:before = a:startline > 1 ? l:lines[ : a:startline - 2] : [] - - let l:full_formatted = l:before + l:formatted + l:lines[a:endline :] - call maktaba#buffer#Overwrite(1, line('$'), l:full_formatted) + " dartfmt does not support range formatting yet: + " https://github.com/dart-lang/dart_style/issues/92 + call codefmt#formatterhelpers#AttemptFakeRangeFormatting( + \ a:startline, a:endline, maktaba#syscall#Create(l:cmd)) catch /ERROR(ShellError):/ " Parse all the errors and stick them in the quickfix list. let l:errors = [] @@ -71,7 +61,8 @@ function! codefmt#dartfmt#GetFormatter() abort if empty(l:errors) " Couldn't parse dartfmt error format; display it all. - call maktaba#error#Shout('Failed to format range; showing all errors: %s', v:exception) + call maktaba#error#Shout( + \ 'Failed to format range; showing all errors: %s', v:exception) else let l:errorHeaderLines = split(v:exception, "\n")[1 : 5] let l:errorHeader = join(l:errorHeaderLines, "\n") diff --git a/autoload/codefmt/formatterhelpers.vim b/autoload/codefmt/formatterhelpers.vim new file mode 100644 index 0000000..c725ee6 --- /dev/null +++ b/autoload/codefmt/formatterhelpers.vim @@ -0,0 +1,59 @@ +" Copyright 2017 Google Inc. All rights reserved. +" +" Licensed under the Apache License, Version 2.0 (the "License"); +" you may not use this file except in compliance with the License. +" You may obtain a copy of the License at +" +" http://www.apache.org/licenses/LICENSE-2.0 +" +" Unless required by applicable law or agreed to in writing, software +" distributed under the License is distributed on an "AS IS" BASIS, +" WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +" See the License for the specific language governing permissions and +" limitations under the License. + + +"" +" @public +" Format lines in the current buffer via a formatter invoked by {cmd}. The +" command includes the explicit range line numbers to use, if any. +" +" @throws ShellError if the {cmd} system call fails +function! codefmt#formatterhelpers#Format(cmd) abort + let l:lines = getline(1, line('$')) + let l:input = join(l:lines, "\n") + + let l:result = maktaba#syscall#Create(a:cmd).WithStdin(l:input).Call() + let l:formatted = split(l:result.stdout, "\n") + + call maktaba#buffer#Overwrite(1, line('$'), l:formatted) +endfunction + +"" +" @public +" Attempt to format a range of lines from {startline} to {endline} in the +" current buffer via a formatter that doesn't natively support range +" formatting (invoked by {cmd}), using a hacky strategy of sending those lines +" to the formatter in isolation. +" +" If invoking this hack, please make sure to file a feature request against +" the tool for range formatting and post a URL for that feature request above +" code that calls it. +" +" @throws ShellError if the {cmd} system call fails +function! codefmt#formatterhelpers#AttemptFakeRangeFormatting( + \ startline, endline, cmd) abort + call maktaba#ensure#IsNumber(a:startline) + call maktaba#ensure#IsNumber(a:endline) + + let l:lines = getline(1, line('$')) + let l:input = join(l:lines[a:startline - 1 : a:endline - 1], "\n") + + let l:result = a:cmd.WithStdin(l:input).Call() + let l:formatted = split(l:result.stdout, "\n") + " Special case empty slice: neither l:lines[:0] nor l:lines[:-1] is right. + let l:before = a:startline > 1 ? l:lines[ : a:startline - 2] : [] + let l:full_formatted = l:before + l:formatted + l:lines[a:endline :] + + call maktaba#buffer#Overwrite(1, line('$'), l:full_formatted) +endfunction diff --git a/autoload/codefmt/gofmt.vim b/autoload/codefmt/gofmt.vim index bab8317..43ffec7 100644 --- a/autoload/codefmt/gofmt.vim +++ b/autoload/codefmt/gofmt.vim @@ -38,20 +38,12 @@ function! codefmt#gofmt#GetFormatter() abort " @flag(gofmt_executable), only targeting the range between {startline} and " {endline}. function l:formatter.FormatRange(startline, endline) abort - " Hack range formatting by formatting range individually, ignoring context. let l:cmd = [ s:plugin.Flag('gofmt_executable') ] - call maktaba#ensure#IsNumber(a:startline) - call maktaba#ensure#IsNumber(a:endline) - let l:lines = getline(1, line('$')) - let l:input = join(l:lines[a:startline - 1 : a:endline - 1], "\n") try - let l:result = maktaba#syscall#Create(l:cmd).WithStdin(l:input).Call() - let l:formatted = split(l:result.stdout, "\n") - " Special case empty slice: neither l:lines[:0] nor l:lines[:-1] is right. - let l:before = a:startline > 1 ? l:lines[ : a:startline - 2] : [] - - let l:full_formatted = l:before + l:formatted + l:lines[a:endline :] - call maktaba#buffer#Overwrite(1, line('$'), l:full_formatted) + " gofmt does not support range formatting. + " TODO: File a feature request with gofmt and link it here. + call codefmt#formatterhelpers#AttemptFakeRangeFormatting( + \ a:startline, a:endline, maktaba#syscall#Create(l:cmd)) catch /ERROR(ShellError):/ " Parse all the errors and stick them in the quickfix list. let l:errors = [] diff --git a/autoload/codefmt/googlejava.vim b/autoload/codefmt/googlejava.vim index 618f0e7..ddffe5a 100644 --- a/autoload/codefmt/googlejava.vim +++ b/autoload/codefmt/googlejava.vim @@ -59,10 +59,7 @@ function! codefmt#googlejava#GetFormatter() abort let l:ranges_str = join(map(copy(a:ranges), 'v:val[0] . ":" . v:val[1]'), ',') let l:cmd += ['--lines', l:ranges_str, '-'] - let l:input = join(getline(1, line('$')), "\n") - let l:result = maktaba#syscall#Create(l:cmd).WithStdin(l:input).Call() - let l:formatted = split(l:result.stdout, "\n") - call maktaba#buffer#Overwrite(1, line('$'), l:formatted) + call codefmt#formatterhelpers#Format(maktaba#syscall#Create(l:cmd)) endfunction return l:formatter diff --git a/autoload/codefmt/jsbeautify.vim b/autoload/codefmt/jsbeautify.vim index 0bd6fdc..ce07fcc 100644 --- a/autoload/codefmt/jsbeautify.vim +++ b/autoload/codefmt/jsbeautify.vim @@ -52,17 +52,10 @@ function! codefmt#jsbeautify#GetFormatter() abort call maktaba#ensure#IsNumber(a:startline) call maktaba#ensure#IsNumber(a:endline) - let l:lines = getline(1, line('$')) - " Hack range formatting by formatting range individually, ignoring context. - let l:input = join(l:lines[a:startline - 1 : a:endline - 1], "\n") - - let l:result = maktaba#syscall#Create(l:cmd).WithStdin(l:input).Call() - let l:formatted = split(l:result.stdout, "\n") - " Special case empty slice: neither l:lines[:0] nor l:lines[:-1] is right. - let l:before = a:startline > 1 ? l:lines[ : a:startline - 2] : [] - let l:full_formatted = l:before + l:formatted + l:lines[a:endline :] - - call maktaba#buffer#Overwrite(1, line('$'), l:full_formatted) + " js-beautify does not support range formatting yet: + " https://github.com/beautify-web/js-beautify/issues/610 + call codefmt#formatterhelpers#AttemptFakeRangeFormatting( + \ a:startline, a:endline, maktaba#syscall#Create(l:cmd)) endfunction return l:formatter diff --git a/autoload/codefmt/rustfmt.vim b/autoload/codefmt/rustfmt.vim index 07a48e8..2c0789f 100644 --- a/autoload/codefmt/rustfmt.vim +++ b/autoload/codefmt/rustfmt.vim @@ -52,13 +52,10 @@ function! codefmt#rustfmt#GetFormatter() abort call extend(l:cmd, l:rustfmt_options) try - let l:lines = getline(1, line('$')) - let l:input = join(l:lines, "\n") - let l:result = maktaba#syscall#Create(l:cmd).WithStdin(l:input).Call() - let l:formatted = split(l:result.stdout, "\n") + " NOTE: Ignores any line ranges given and formats entire buffer. " Even though rustfmt supports formatting ranges through the --file-lines " flag, it is not still enabled in the stable binaries. - call maktaba#buffer#Overwrite(1, line('$'), l:formatted) + call codefmt#formatterhelpers#Format(l:cmd) catch /ERROR(ShellError):/ " Parse all the errors and stick them in the quickfix list. let l:errors = [] diff --git a/autoload/codefmt/shfmt.vim b/autoload/codefmt/shfmt.vim index 19845c2..3640d20 100644 --- a/autoload/codefmt/shfmt.vim +++ b/autoload/codefmt/shfmt.vim @@ -48,22 +48,14 @@ function! codefmt#shfmt#GetFormatter() abort \ 'shfmt_options flag must be list or callable. Found %s', \ string(l:Shfmt_options)) endif - " Hack range formatting by formatting range individually, ignoring context. - " Feature request for range formatting: - " https://github.com/mvdan/sh/issues/333 let l:cmd = [ s:plugin.Flag('shfmt_executable') ] + l:shfmt_options - call maktaba#ensure#IsNumber(a:startline) - call maktaba#ensure#IsNumber(a:endline) - let l:lines = getline(1, line('$')) - let l:input = join(l:lines[a:startline - 1 : a:endline - 1], "\n") try - let l:result = maktaba#syscall#Create(l:cmd).WithStdin(l:input).Call() - let l:formatted = split(l:result.stdout, "\n") - " Special case empty slice: neither l:lines[:0] nor l:lines[:-1] is right. - let l:before = a:startline > 1 ? l:lines[ : a:startline - 2] : [] - - let l:full_formatted = l:before + l:formatted + l:lines[a:endline :] - call maktaba#buffer#Overwrite(1, line('$'), l:full_formatted) + " Feature request for range formatting: + " https://github.com/mvdan/sh/issues/333 + call codefmt#formatterhelpers#AttemptFakeRangeFormatting( + \ a:startline, + \ a:endline, + \ maktaba#syscall#Create(l:cmd)) catch /ERROR(ShellError):/ " Parse all the errors and stick them in the quickfix list. let l:errors = [] diff --git a/autoload/codefmt/zprint.vim b/autoload/codefmt/zprint.vim index db0af6d..bc0a287 100644 --- a/autoload/codefmt/zprint.vim +++ b/autoload/codefmt/zprint.vim @@ -63,28 +63,20 @@ function! codefmt#zprint#GetFormatter() abort \ 'zprint_options flag must be list or callable. Found %s', \ string(l:ZprintOptions)) endif - let l:cmd = [s:plugin.Flag('zprint_executable')] - call extend(l:cmd, l:zprint_options) - - call maktaba#ensure#IsNumber(a:startline) - call maktaba#ensure#IsNumber(a:endline) - let l:lines = getline(1, line('$')) - - " zprint doesn't support formatting a range of lines, so format the range - " individually, ignoring context. This works well for top-level forms, although it's - " not ideal for inner forms because it loses the indentation. - let l:input = join(l:lines[a:startline - 1 : a:endline - 1], "\n") + let l:cmd_args = [s:plugin.Flag('zprint_executable')] + call extend(l:cmd_args, l:zprint_options) " Prepare the syscall, changing to the containing directory in case the user " has configured {:search-config? true} in ~/.zprintrc - let l:result = maktaba#syscall#Create(l:cmd).WithCwd(expand('%:p:h')).WithStdin(l:input).Call() - let l:formatted = split(l:result.stdout, "\n") - - " Special case empty slice: neither l:lines[:0] nor l:lines[:-1] is right. - let l:before = a:startline > 1 ? l:lines[ : a:startline - 2] : [] - let l:full_formatted = l:before + l:formatted + l:lines[a:endline :] - - call maktaba#buffer#Overwrite(1, line('$'), l:full_formatted) + let l:cmd = maktaba#syscall#Create(l:cmd_args).WithCwd(expand('%:p:h')) + " zprint does not support range formatting yet: + " https://github.com/kkinnear/zprint/issues/122 + " This fake range formatting works well for top-level forms, although it's + " not ideal for inner forms because it loses the indentation. + call codefmt#formatterhelpers#AttemptFakeRangeFormatting( + \ a:startline, + \ a:endline, + \ l:cmd) endfunction return l:formatter diff --git a/doc/codefmt.txt b/doc/codefmt.txt index 3c1b455..331f67d 100644 --- a/doc/codefmt.txt +++ b/doc/codefmt.txt @@ -225,6 +225,25 @@ codefmt#FormatMap({type}) *codefmt#FormatMap()* Suitable for use as 'operatorfunc'; see |g@| for details. The type is ignored since formatting only works on complete lines. +codefmt#formatterhelpers#Format({cmd}) *codefmt#formatterhelpers#Format()* + Format lines in the current buffer via a formatter invoked by {cmd}. The + command includes the explicit range line numbers to use, if any. + + Throws ERROR(ShellError) if the {cmd} system call fails + +codefmt#formatterhelpers#AttemptFakeRangeFormatting({startline}, {endline}, + {cmd}) *codefmt#formatterhelpers#AttemptFakeRangeFormatting()* + Attempt to format a range of lines from {startline} to {endline} in the + current buffer via a formatter that doesn't natively support range + formatting (invoked by {cmd}), using a hacky strategy of sending those lines + to the formatter in isolation. + + If invoking this hack, please make sure to file a feature request against + the tool for range formatting and post a URL for that feature request above + code that calls it. + + Throws ERROR(ShellError) if the {cmd} system call fails + ============================================================================== MAPPINGS *codefmt-mappings* From 82af84bee130d9f3faf0eddf5de46d8ed8473f7d Mon Sep 17 00:00:00 2001 From: David Barnett Date: Wed, 8 Jan 2020 07:43:17 -0800 Subject: [PATCH 2/5] Fix broken formatting for buildifier/rustfmt, check Syscall args Clarifies that the formatterhelper functions expect a maktaba.Syscall, enforces that, and fixes two formatters buildifier and rustfmt that didn't. --- autoload/codefmt/buildifier.vim | 2 +- autoload/codefmt/formatterhelpers.vim | 23 +++++++++++++++++++---- autoload/codefmt/rustfmt.vim | 2 +- doc/codefmt.txt | 9 +++++---- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/autoload/codefmt/buildifier.vim b/autoload/codefmt/buildifier.vim index ff964f3..41eca45 100644 --- a/autoload/codefmt/buildifier.vim +++ b/autoload/codefmt/buildifier.vim @@ -49,7 +49,7 @@ function! codefmt#buildifier#GetFormatter() abort try " NOTE: Ignores any line ranges given and formats entire buffer. " buildifier does not support range formatting. - call codefmt#formatterhelpers#Format(l:cmd) + call codefmt#formatterhelpers#Format(maktaba#syscall#Create(l:cmd)) catch " Parse all the errors and stick them in the quickfix list. let l:errors = [] diff --git a/autoload/codefmt/formatterhelpers.vim b/autoload/codefmt/formatterhelpers.vim index c725ee6..4063851 100644 --- a/autoload/codefmt/formatterhelpers.vim +++ b/autoload/codefmt/formatterhelpers.vim @@ -13,13 +13,27 @@ " limitations under the License. +function! s:EnsureIsSyscall(Value) abort + if type(a:Value) == type({}) && + \ has_key(a:Value, 'Call') && + \ maktaba#function#HasSameName( + \ a:Value.Call, function('maktaba#syscall#Call')) + return a:Value + endif + throw maktaba#error#BadValue( + \ 'Not a valid matkaba.Syscall: %s', string(a:Value)) +endfunction + + "" " @public -" Format lines in the current buffer via a formatter invoked by {cmd}. The -" command includes the explicit range line numbers to use, if any. +" Format lines in the current buffer via a formatter invoked by {cmd} (a +" |maktaba.Syscall|). The command includes the explicit range line numbers to +" use, if any. " " @throws ShellError if the {cmd} system call fails function! codefmt#formatterhelpers#Format(cmd) abort + call s:EnsureIsSyscall(a:cmd) let l:lines = getline(1, line('$')) let l:input = join(l:lines, "\n") @@ -33,8 +47,8 @@ endfunction " @public " Attempt to format a range of lines from {startline} to {endline} in the " current buffer via a formatter that doesn't natively support range -" formatting (invoked by {cmd}), using a hacky strategy of sending those lines -" to the formatter in isolation. +" formatting (invoked by {cmd}, a |maktaba.Syscall|), using a hacky strategy +" of sending those lines to the formatter in isolation. " " If invoking this hack, please make sure to file a feature request against " the tool for range formatting and post a URL for that feature request above @@ -45,6 +59,7 @@ function! codefmt#formatterhelpers#AttemptFakeRangeFormatting( \ startline, endline, cmd) abort call maktaba#ensure#IsNumber(a:startline) call maktaba#ensure#IsNumber(a:endline) + call s:EnsureIsSyscall(a:cmd) let l:lines = getline(1, line('$')) let l:input = join(l:lines[a:startline - 1 : a:endline - 1], "\n") diff --git a/autoload/codefmt/rustfmt.vim b/autoload/codefmt/rustfmt.vim index 2c0789f..73d4582 100644 --- a/autoload/codefmt/rustfmt.vim +++ b/autoload/codefmt/rustfmt.vim @@ -55,7 +55,7 @@ function! codefmt#rustfmt#GetFormatter() abort " NOTE: Ignores any line ranges given and formats entire buffer. " Even though rustfmt supports formatting ranges through the --file-lines " flag, it is not still enabled in the stable binaries. - call codefmt#formatterhelpers#Format(l:cmd) + call codefmt#formatterhelpers#Format(maktaba#syscall#Create(l:cmd)) catch /ERROR(ShellError):/ " Parse all the errors and stick them in the quickfix list. let l:errors = [] diff --git a/doc/codefmt.txt b/doc/codefmt.txt index 331f67d..8f36fbb 100644 --- a/doc/codefmt.txt +++ b/doc/codefmt.txt @@ -226,8 +226,9 @@ codefmt#FormatMap({type}) *codefmt#FormatMap()* ignored since formatting only works on complete lines. codefmt#formatterhelpers#Format({cmd}) *codefmt#formatterhelpers#Format()* - Format lines in the current buffer via a formatter invoked by {cmd}. The - command includes the explicit range line numbers to use, if any. + Format lines in the current buffer via a formatter invoked by {cmd} (a + |maktaba.Syscall|). The command includes the explicit range line numbers to + use, if any. Throws ERROR(ShellError) if the {cmd} system call fails @@ -235,8 +236,8 @@ codefmt#formatterhelpers#AttemptFakeRangeFormatting({startline}, {endline}, {cmd}) *codefmt#formatterhelpers#AttemptFakeRangeFormatting()* Attempt to format a range of lines from {startline} to {endline} in the current buffer via a formatter that doesn't natively support range - formatting (invoked by {cmd}), using a hacky strategy of sending those lines - to the formatter in isolation. + formatting (invoked by {cmd}, a |maktaba.Syscall|), using a hacky strategy + of sending those lines to the formatter in isolation. If invoking this hack, please make sure to file a feature request against the tool for range formatting and post a URL for that feature request above From a1c0f71a41c67fad0920ba2a1d77d52bb09ef3a7 Mon Sep 17 00:00:00 2001 From: David Barnett Date: Wed, 8 Jan 2020 07:43:43 -0800 Subject: [PATCH 3/5] Review nits: unused variable and old copyright year --- autoload/codefmt/buildifier.vim | 1 - autoload/codefmt/formatterhelpers.vim | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/autoload/codefmt/buildifier.vim b/autoload/codefmt/buildifier.vim index 41eca45..efc0194 100644 --- a/autoload/codefmt/buildifier.vim +++ b/autoload/codefmt/buildifier.vim @@ -45,7 +45,6 @@ function! codefmt#buildifier#GetFormatter() abort let l:cmd += ['-path', l:fname] endif - let l:input = join(getline(1, line('$')), "\n") try " NOTE: Ignores any line ranges given and formats entire buffer. " buildifier does not support range formatting. diff --git a/autoload/codefmt/formatterhelpers.vim b/autoload/codefmt/formatterhelpers.vim index 4063851..0ce4bd8 100644 --- a/autoload/codefmt/formatterhelpers.vim +++ b/autoload/codefmt/formatterhelpers.vim @@ -1,4 +1,4 @@ -" Copyright 2017 Google Inc. All rights reserved. +" Copyright 2020 Google Inc. All rights reserved. " " Licensed under the Apache License, Version 2.0 (the "License"); " you may not use this file except in compliance with the License. From b9a4108505217559b36e0f3f07541380b194852d Mon Sep 17 00:00:00 2001 From: David Barnett Date: Wed, 8 Jan 2020 08:40:03 -0800 Subject: [PATCH 4/5] Change helpers back to accept syscall-like args, but consistently --- autoload/codefmt/autopep8.vim | 6 +++--- autoload/codefmt/buildifier.vim | 2 +- autoload/codefmt/dartfmt.vim | 2 +- autoload/codefmt/formatterhelpers.vim | 30 +++++++++------------------ autoload/codefmt/gofmt.vim | 2 +- autoload/codefmt/googlejava.vim | 2 +- autoload/codefmt/jsbeautify.vim | 2 +- autoload/codefmt/rustfmt.vim | 2 +- autoload/codefmt/shfmt.vim | 2 +- doc/codefmt.txt | 14 ++++++++----- 10 files changed, 29 insertions(+), 35 deletions(-) diff --git a/autoload/codefmt/autopep8.vim b/autoload/codefmt/autopep8.vim index 40c5db1..6b42b8c 100644 --- a/autoload/codefmt/autopep8.vim +++ b/autoload/codefmt/autopep8.vim @@ -69,15 +69,15 @@ function! codefmt#autopep8#GetFormatter() abort call maktaba#ensure#IsNumber(a:endline) if s:autopep8_supports_range - call codefmt#formatterhelpers#Format(maktaba#syscall#Create([ + call codefmt#formatterhelpers#Format([ \ l:executable, \ '--range', string(a:startline), string(a:endline), - \ '-'])) + \ '-']) else call codefmt#formatterhelpers#AttemptFakeRangeFormatting( \ a:startline, \ a:endline, - \ maktaba#syscall#Create([l:executable, '-'])) + \ [l:executable, '-']) endif endfunction diff --git a/autoload/codefmt/buildifier.vim b/autoload/codefmt/buildifier.vim index efc0194..743df9a 100644 --- a/autoload/codefmt/buildifier.vim +++ b/autoload/codefmt/buildifier.vim @@ -48,7 +48,7 @@ function! codefmt#buildifier#GetFormatter() abort try " NOTE: Ignores any line ranges given and formats entire buffer. " buildifier does not support range formatting. - call codefmt#formatterhelpers#Format(maktaba#syscall#Create(l:cmd)) + call codefmt#formatterhelpers#Format(l:cmd) catch " Parse all the errors and stick them in the quickfix list. let l:errors = [] diff --git a/autoload/codefmt/dartfmt.vim b/autoload/codefmt/dartfmt.vim index f0ed5c6..f2c0022 100644 --- a/autoload/codefmt/dartfmt.vim +++ b/autoload/codefmt/dartfmt.vim @@ -43,7 +43,7 @@ function! codefmt#dartfmt#GetFormatter() abort " dartfmt does not support range formatting yet: " https://github.com/dart-lang/dart_style/issues/92 call codefmt#formatterhelpers#AttemptFakeRangeFormatting( - \ a:startline, a:endline, maktaba#syscall#Create(l:cmd)) + \ a:startline, a:endline, l:cmd) catch /ERROR(ShellError):/ " Parse all the errors and stick them in the quickfix list. let l:errors = [] diff --git a/autoload/codefmt/formatterhelpers.vim b/autoload/codefmt/formatterhelpers.vim index 0ce4bd8..7ee5a00 100644 --- a/autoload/codefmt/formatterhelpers.vim +++ b/autoload/codefmt/formatterhelpers.vim @@ -13,27 +13,15 @@ " limitations under the License. -function! s:EnsureIsSyscall(Value) abort - if type(a:Value) == type({}) && - \ has_key(a:Value, 'Call') && - \ maktaba#function#HasSameName( - \ a:Value.Call, function('maktaba#syscall#Call')) - return a:Value - endif - throw maktaba#error#BadValue( - \ 'Not a valid matkaba.Syscall: %s', string(a:Value)) -endfunction - - "" " @public -" Format lines in the current buffer via a formatter invoked by {cmd} (a -" |maktaba.Syscall|). The command includes the explicit range line numbers to -" use, if any. +" Format lines in the current buffer via a formatter invoked by {cmd}, which +" is a system call represented by either a |maktaba.Syscall| or any argument +" accepted by |maktaba#syscall#Create()|. The command includes any arguments +" for the explicit range line numbers to use, if any. " " @throws ShellError if the {cmd} system call fails function! codefmt#formatterhelpers#Format(cmd) abort - call s:EnsureIsSyscall(a:cmd) let l:lines = getline(1, line('$')) let l:input = join(l:lines, "\n") @@ -47,8 +35,11 @@ endfunction " @public " Attempt to format a range of lines from {startline} to {endline} in the " current buffer via a formatter that doesn't natively support range -" formatting (invoked by {cmd}, a |maktaba.Syscall|), using a hacky strategy -" of sending those lines to the formatter in isolation. +" formatting, which is invoked via {cmd} (a system call represented by either +" a |maktaba.Syscall| or any argument accepted by |maktaba#syscall#Create()|). +" It uses a hacky strategy of sending those lines to the formatter in +" isolation, which gives bad results if the code on those lines isn't +" a self-contained block of syntax or is part of a larger indent. " " If invoking this hack, please make sure to file a feature request against " the tool for range formatting and post a URL for that feature request above @@ -59,12 +50,11 @@ function! codefmt#formatterhelpers#AttemptFakeRangeFormatting( \ startline, endline, cmd) abort call maktaba#ensure#IsNumber(a:startline) call maktaba#ensure#IsNumber(a:endline) - call s:EnsureIsSyscall(a:cmd) let l:lines = getline(1, line('$')) let l:input = join(l:lines[a:startline - 1 : a:endline - 1], "\n") - let l:result = a:cmd.WithStdin(l:input).Call() + let l:result = maktaba#syscall#Create(a:cmd).WithStdin(l:input).Call() let l:formatted = split(l:result.stdout, "\n") " Special case empty slice: neither l:lines[:0] nor l:lines[:-1] is right. let l:before = a:startline > 1 ? l:lines[ : a:startline - 2] : [] diff --git a/autoload/codefmt/gofmt.vim b/autoload/codefmt/gofmt.vim index 43ffec7..da4aad6 100644 --- a/autoload/codefmt/gofmt.vim +++ b/autoload/codefmt/gofmt.vim @@ -43,7 +43,7 @@ function! codefmt#gofmt#GetFormatter() abort " gofmt does not support range formatting. " TODO: File a feature request with gofmt and link it here. call codefmt#formatterhelpers#AttemptFakeRangeFormatting( - \ a:startline, a:endline, maktaba#syscall#Create(l:cmd)) + \ a:startline, a:endline, l:cmd) catch /ERROR(ShellError):/ " Parse all the errors and stick them in the quickfix list. let l:errors = [] diff --git a/autoload/codefmt/googlejava.vim b/autoload/codefmt/googlejava.vim index ddffe5a..94fba8e 100644 --- a/autoload/codefmt/googlejava.vim +++ b/autoload/codefmt/googlejava.vim @@ -59,7 +59,7 @@ function! codefmt#googlejava#GetFormatter() abort let l:ranges_str = join(map(copy(a:ranges), 'v:val[0] . ":" . v:val[1]'), ',') let l:cmd += ['--lines', l:ranges_str, '-'] - call codefmt#formatterhelpers#Format(maktaba#syscall#Create(l:cmd)) + call codefmt#formatterhelpers#Format(l:cmd) endfunction return l:formatter diff --git a/autoload/codefmt/jsbeautify.vim b/autoload/codefmt/jsbeautify.vim index ce07fcc..f7f706c 100644 --- a/autoload/codefmt/jsbeautify.vim +++ b/autoload/codefmt/jsbeautify.vim @@ -55,7 +55,7 @@ function! codefmt#jsbeautify#GetFormatter() abort " js-beautify does not support range formatting yet: " https://github.com/beautify-web/js-beautify/issues/610 call codefmt#formatterhelpers#AttemptFakeRangeFormatting( - \ a:startline, a:endline, maktaba#syscall#Create(l:cmd)) + \ a:startline, a:endline, l:cmd) endfunction return l:formatter diff --git a/autoload/codefmt/rustfmt.vim b/autoload/codefmt/rustfmt.vim index 73d4582..2c0789f 100644 --- a/autoload/codefmt/rustfmt.vim +++ b/autoload/codefmt/rustfmt.vim @@ -55,7 +55,7 @@ function! codefmt#rustfmt#GetFormatter() abort " NOTE: Ignores any line ranges given and formats entire buffer. " Even though rustfmt supports formatting ranges through the --file-lines " flag, it is not still enabled in the stable binaries. - call codefmt#formatterhelpers#Format(maktaba#syscall#Create(l:cmd)) + call codefmt#formatterhelpers#Format(l:cmd) catch /ERROR(ShellError):/ " Parse all the errors and stick them in the quickfix list. let l:errors = [] diff --git a/autoload/codefmt/shfmt.vim b/autoload/codefmt/shfmt.vim index 3640d20..c701731 100644 --- a/autoload/codefmt/shfmt.vim +++ b/autoload/codefmt/shfmt.vim @@ -55,7 +55,7 @@ function! codefmt#shfmt#GetFormatter() abort call codefmt#formatterhelpers#AttemptFakeRangeFormatting( \ a:startline, \ a:endline, - \ maktaba#syscall#Create(l:cmd)) + \ l:cmd) catch /ERROR(ShellError):/ " Parse all the errors and stick them in the quickfix list. let l:errors = [] diff --git a/doc/codefmt.txt b/doc/codefmt.txt index 8f36fbb..fb79235 100644 --- a/doc/codefmt.txt +++ b/doc/codefmt.txt @@ -226,9 +226,10 @@ codefmt#FormatMap({type}) *codefmt#FormatMap()* ignored since formatting only works on complete lines. codefmt#formatterhelpers#Format({cmd}) *codefmt#formatterhelpers#Format()* - Format lines in the current buffer via a formatter invoked by {cmd} (a - |maktaba.Syscall|). The command includes the explicit range line numbers to - use, if any. + Format lines in the current buffer via a formatter invoked by {cmd}, which + is a system call represented by either a |maktaba.Syscall| or any argument + accepted by |maktaba#syscall#Create()|. The command includes any arguments + for the explicit range line numbers to use, if any. Throws ERROR(ShellError) if the {cmd} system call fails @@ -236,8 +237,11 @@ codefmt#formatterhelpers#AttemptFakeRangeFormatting({startline}, {endline}, {cmd}) *codefmt#formatterhelpers#AttemptFakeRangeFormatting()* Attempt to format a range of lines from {startline} to {endline} in the current buffer via a formatter that doesn't natively support range - formatting (invoked by {cmd}, a |maktaba.Syscall|), using a hacky strategy - of sending those lines to the formatter in isolation. + formatting, which is invoked via {cmd} (a system call represented by either + a |maktaba.Syscall| or any argument accepted by |maktaba#syscall#Create()|). + It uses a hacky strategy of sending those lines to the formatter in + isolation, which gives bad results if the code on those lines isn't a + self-contained block of syntax or is part of a larger indent. If invoking this hack, please make sure to file a feature request against the tool for range formatting and post a URL for that feature request above From c09d63b0cadda8b868bb73dead3657a251d06f0f Mon Sep 17 00:00:00 2001 From: David Barnett Date: Wed, 8 Jan 2020 09:07:44 -0800 Subject: [PATCH 5/5] =?UTF-8?q?Wording=20nit=20from=20review:=20"command?= =?UTF-8?q?=20includes"=20=E2=86=92=20"command=20must=20include"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- autoload/codefmt/formatterhelpers.vim | 4 ++-- doc/codefmt.txt | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/autoload/codefmt/formatterhelpers.vim b/autoload/codefmt/formatterhelpers.vim index 7ee5a00..43f06c9 100644 --- a/autoload/codefmt/formatterhelpers.vim +++ b/autoload/codefmt/formatterhelpers.vim @@ -17,8 +17,8 @@ " @public " Format lines in the current buffer via a formatter invoked by {cmd}, which " is a system call represented by either a |maktaba.Syscall| or any argument -" accepted by |maktaba#syscall#Create()|. The command includes any arguments -" for the explicit range line numbers to use, if any. +" accepted by |maktaba#syscall#Create()|. The command must include any +" arguments for the explicit range line numbers to use, if any. " " @throws ShellError if the {cmd} system call fails function! codefmt#formatterhelpers#Format(cmd) abort diff --git a/doc/codefmt.txt b/doc/codefmt.txt index fb79235..4f55739 100644 --- a/doc/codefmt.txt +++ b/doc/codefmt.txt @@ -228,8 +228,8 @@ codefmt#FormatMap({type}) *codefmt#FormatMap()* codefmt#formatterhelpers#Format({cmd}) *codefmt#formatterhelpers#Format()* Format lines in the current buffer via a formatter invoked by {cmd}, which is a system call represented by either a |maktaba.Syscall| or any argument - accepted by |maktaba#syscall#Create()|. The command includes any arguments - for the explicit range line numbers to use, if any. + accepted by |maktaba#syscall#Create()|. The command must include any + arguments for the explicit range line numbers to use, if any. Throws ERROR(ShellError) if the {cmd} system call fails