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

[RDY] Allow multiple function definitions in one :execute #6914

Merged
merged 7 commits into from Jun 21, 2017

Conversation

Projects
None yet
4 participants
@ZyX-I
Contributor

ZyX-I commented Jun 20, 2017

It appeared to be easy enough to not error out, but actually allow commands after :endfunction.

Ref #6844.

@ZyX-I ZyX-I force-pushed the ZyX-I:func-def-trailing-error branch from 11a61d8 to 40f98b1 Jun 20, 2017

eval: Allow running next command after :endfunction
This will still error out on `:endfunction | next`, but defining many functions
in one `:execute` should be possible.

@ZyX-I ZyX-I force-pushed the ZyX-I:func-def-trailing-error branch from 40f98b1 to 309fcfc Jun 20, 2017

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Jun 20, 2017

Having a bunch of failures like

# test/functional/eval/system_spec.lua @ 167
10:58:54,397 INFO  - # Failure message: ./test/functional/ui/screen.lua:301: Row 1 did not match.
10:58:54,397 INFO  - # Expected:
10:58:54,397 INFO  - # |* ^ |
10:58:57,921 INFO  - # | ~ |
10:58:57,921 INFO  - # | ~ |
10:58:57,921 INFO  - # | ~ |
10:58:57,921 INFO  - # | ~ |
10:58:57,921 INFO  - # | ~ |
10:58:57,921 INFO  - # | ~ |
10:58:57,921 INFO  - # | ~ |
10:58:57,921 INFO  - # | ~ |
10:58:57,921 INFO  - # | ~ |
10:58:57,921 INFO  - # | ~ |
10:58:57,921 INFO  - # | ~ |
10:58:57,921 INFO  - # | ~ |
10:58:57,921 INFO  - # | :call system("echo") |
10:58:57,921 INFO  - # Actual:
10:58:57,921 INFO  - # |*^ |
10:58:57,921 INFO  - # |~ |
10:58:57,921 INFO  - # |~ |
10:58:57,921 INFO  - # |~ |
10:58:57,921 INFO  - # |~ |
10:58:57,921 INFO  - # |~ |
10:58:57,921 INFO  - # |~ |
10:58:57,921 INFO  - # |~ |
10:58:57,921 INFO  - # |~ |
10:58:57,921 INFO  - # |~ |
10:58:57,921 INFO  - # |~ |
10:58:57,921 INFO  - # |~ |
10:58:57,921 INFO  - # |~ |
10:58:57,921 INFO  - # |:call system("echo") |

in QB and travis (space difference), weird.

@ZyX-I ZyX-I force-pushed the ZyX-I:func-def-trailing-error branch from 309fcfc to 2318e18 Jun 20, 2017

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Jun 20, 2017

Ah, gsub returns more then one value to be passed to dedent.

@ZyX-I ZyX-I force-pushed the ZyX-I:func-def-trailing-error branch from 41aabd1 to 0be76d0 Jun 20, 2017

end)
end)
-- vim: foldmarker=▶,▲

This comment has been minimized.

@justinmk

justinmk Jun 20, 2017

Member

is this intentional?

This comment has been minimized.

@ZyX-I

ZyX-I Jun 21, 2017

Contributor

There are a number of }}} comments. Initially also {{{, but they appeared not to work. For that I made sure that foldmarkers in tests will not actually be considered fold markers by having this modeline with unicode foldmarkers I frequently use.

-- strip it from the remaining lines
str = str:gsub('[\n]'..indent, '\n')
str = str:gsub('[\n]'..indent, '\n' .. left_indent)

This comment has been minimized.

@justinmk

justinmk Jun 20, 2017

Member

why is left_indent is catenated with the string?

@@ -297,12 +297,13 @@ local function dedent(str)
-- no minimum common indent
return str
end
local left_indent = (' '):rep(leave_indent or 0)

This comment has been minimized.

@justinmk

justinmk Jun 20, 2017

Member

With gsub(..., 0) nothing happens. What is the purpose of calling dedent() if the indent is not un-indented?

This comment has been minimized.

@ZyX-I

ZyX-I Jun 21, 2017

Contributor

I added additional argument to add some indent. The purpose is that I may have six-space indent in the function and yet only three-space indent coming out of dedent.

@justinmk

This comment has been minimized.

Member

justinmk commented Jun 20, 2017

Should there be a mention in vim_diff.txt?

testlint issue:

Checking /home/travis/build/neovim/neovim/test/functional/viml/function_spec.lua 1 warning
    /home/travis/build/neovim/neovim/test/functional/viml/function_spec.lua:11:36: unused argument body
@bfredl

This comment has been minimized.

Member

bfredl commented Jun 21, 2017

@justinmk maybe let see if bram accepts it before adding to vim_diff.txt https://groups.google.com/forum/#!topic/vim_dev/oNMz-9TPyFQ

@ZyX-I ZyX-I force-pushed the ZyX-I:func-def-trailing-error branch from 0be76d0 to 476c28f Jun 21, 2017

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Jun 21, 2017

AppVeyor failure seems unrelated:

[  FAILED  ] 1 test, listed below:
[  FAILED  ] C:/projects/neovim/test/functional\core\job_spec.lua @ 45: jobs uses &shell and &shellcmdflag if passed a string
C:/projects/neovim/test/functional\core\job_spec.lua:52: Expected objects to be the same.
Passed in:
(table) {
  [1] = 'notification'
  [2] = 'stdout'
 *[3] = {
    [1] = 0
   *[2] = {
      [1] = 'abc' } } }
Expected:
(table) {
  [1] = 'notification'
  [2] = 'stdout'
 *[3] = {
    [1] = 0
   *[2] = {
      [1] = 'abc'
     *[2] = '' } } }
stack traceback:
	C:/projects/neovim/test/functional\core\job_spec.lua:52: in function <C:/projects/neovim/test/functional\core\job_spec.lua:45>
@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Jun 21, 2017

QB failures are due to “timeout” and “cancel”, in both cases at

04:14:01,853 INFO  - # test/functional/api/keymap_spec.lua @ 215: get_keymap returns script numbers for global maps

Looks unrelated as well. So this PR may be considered ready.

@ZyX-I ZyX-I changed the title from Allow multiple function definitions in one :execute to [RDY] Allow multiple function definitions in one :execute Jun 21, 2017

@marvim marvim added the RDY label Jun 21, 2017

@justinmk justinmk merged commit 144f584 into neovim:master Jun 21, 2017

2 of 4 checks passed

QuickBuild Build pr-6914 finished with status FAILED
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.02%) to 76.009%
Details

@justinmk justinmk removed the RDY label Jun 21, 2017

@ZyX-I ZyX-I deleted the ZyX-I:func-def-trailing-error branch Jun 22, 2017

@KillTheMule KillTheMule referenced this pull request Oct 30, 2017

Merged

[RFC] News #153

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment