Fix: can add the new value to "a:" dict by ichizok · Pull Request #3929 · vim/vim

ichizok · 2019-02-10T18:58:33Z

Currently we can add the new key-value to "a:" dict.

func s:f() abort
  " no error?
  let a:x = 1
endfunc
call s:f()

NOTE: A part of the tests is based on @mattn's code.

mattn · 2019-02-10T19:11:55Z

src/testdir/test_let.vim

+func s:set_arg9(a) abort
+  let a:['b'] = 1
+endfunction
+


please add

function s:set_arg10(...) abort let a:000[0][0] = 1 endfunction call s:set_arg10([0])

Thank you. I added Test_let_varg_fail() for that.

codecov-io · 2019-02-10T19:20:21Z

Codecov Report

Merging #3929 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3929      +/-   ##
==========================================
+ Coverage   78.84%   78.85%   +<.01%     
==========================================
  Files         105      105              
  Lines      142076   142078       +2     
==========================================
+ Hits       112024   112030       +6     
+ Misses      30052    30048       -4

Impacted Files	Coverage Δ
src/eval.c	`85.25% <100%> (+0.11%)`	⬆️
src/userfunc.c	`87.85% <100%> (ø)`	⬆️
src/version.c	`85.39% <0%> (-0.89%)`	⬇️
src/ex_cmds2.c	`84.89% <0%> (-0.1%)`	⬇️
src/ui.c	`49.92% <0%> (-0.08%)`	⬇️
src/gui.c	`58% <0%> (-0.06%)`	⬇️
src/terminal.c	`75.12% <0%> (-0.05%)`	⬇️
src/window.c	`83.53% <0%> (+0.19%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 18c5632...62c7d54. Read the comment docs.

Yilin-Yang · 2019-02-12T04:05:01Z

Would it be possible to leave this behavior enabled, and to change the documentation instead? Removing the ability to declare variables in the a: namespace eliminates what is (IMO) a clean and idiomatic way to use "default arguments" in VimL functions:

""
" @function Foo([bar] [quo])
" @default bar='bar'
" @default quo=2
function! Foo(...) abort
  let a:bar = get(a:000, 0, 'bar')
  let a:quo = get(a:000, 1, 2)
  " ...
endfunction

In a future refactoring, this could be easily changed to:

function! Foo(bar, quo) abort
  " ...

Without needing to find-and-replace l:bar with a:bar, etc. in the function body.

Using the l: scope definitely wouldn't be a huge inconvenience, but I feel that explicitly declaring variables in the a: namespace is better at communicating intent. It's something that I use in my own plugins, at the very least, though the only other plugins I've found that do this declare inside of the l: scope instead (e.g. LanguageClient-neovim, ALE).

mattn · 2019-02-12T05:21:54Z

I don't think that it's good way. Documentation says

The a: scope and the variables in it cannot be changed, they are fixed.

function s:foo(b)
  let a:b = 1 " ERROR
  let a:c = 1 " NOT ERROR
endfunction

We can not make documentation correct and non-contradictory if it allow to change.

k-takata · 2019-02-12T11:16:45Z

I'd like to say that the old behavior is inconsistent as @mattn shows in the example.

Closes #148.

brammool · 2019-02-12T20:47:16Z

Would it be possible to leave this behavior enabled, and to change the documentation instead? Removing the ability to declare variables in the `a:` namespace eliminates what is (IMO) a clean and idiomatic way to use "default arguments" in VimL functions: ```vim "" " @function Foo([bar] [quo]) " @default bar='bar' " @default quo=2 function! Foo(...) abort let a:bar = get(a:000, 0, 'bar') let a:quo = get(a:000, 1, 2) " ... endfunction ``` In a future refactoring, this could be easily changed to: ```vim function! Foo(bar, quo) abort " ... ``` Without needing to find-and-replace `l:bar` with `a:bar`, etc. in the function body. Using the `l:` scope definitely wouldn't be a huge inconvenience, but I feel that explicitly declaring variables in the `a:` namespace is better at communicating intent. It's something that I use in [my own plugins,](https://github.com/Yilin-Yang/TypeVim/blob/4b75041aad7de2bf4121679a7631f51c1bc3b657/autoload/typevim/object.vim#L519) at the very least, though the only other plugins I've found that do this declare inside of the `l:` scope instead (e.g. [LanguageClient-neovim](https://github.com/autozimu/LanguageClient-neovim/blob/8eba06bf6540915deeae7da9b239f0f918e20cd9/autoload/LanguageClient.vim#L515), [ALE](https://github.com/w0rp/ale/blob/bf196ba17c9e261e4e3a9dba64260c6d0b2c8af9/autoload/ale.vim#L120)).

It's a basic principle that arguments don't change. It helps a lot for debugging, since you can rely on the a: dict not to have changed somewhere. For optional arguments it's better to follow what most other languages do: func Foo(must_have, optional = value) Thus optional arguments always have a default value. That avoids having to check for whether they are present or not. It would also be possible to allow specifying the argument name in the call: func Foo(must_have, optional = value, also_optional = '') call Foo('must', also_optional = 'there') However, it has the disadvantage that changing the argument name in the function definition requires all calls to be changed, and one doesn't always know what calls use the name, which leads to mistakes. Just using positional arguments is simpler. One can pass a dictionary to have a bunch of optional arguments (as many built-in functions already do).

…

-- Wi n0t trei a h0liday in Sweden thi yer? "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

vim-ml · 2019-02-12T21:09:58Z

On 2/12/19 3:47 PM, Bram Moolenaar (Vim Github Repository) wrote: > Would it be possible to leave this behavior enabled, and to change the > documentation instead? Removing the ability to declare variables in > the `a:` namespace eliminates what is (IMO) a clean and idiomatic way > to use "default arguments" in VimL functions: > > ```vim > "" > " @function Foo([bar] [quo]) > " @default bar='bar' > " @default quo=2 > function! Foo(...) abort > let a:bar = get(a:000, 0, 'bar') > let a:quo = get(a:000, 1, 2) > " ... > endfunction > ``` > > In a future refactoring, this could be easily changed to: > ```vim > function! Foo(bar, quo) abort > " ... > ``` > Without needing to find-and-replace `l:bar` with `a:bar`, etc. in the > function body. > > Using the `l:` scope definitely wouldn't be a huge inconvenience, but > I feel that explicitly declaring variables in the `a:` namespace is > better at communicating intent. It's something that I use in [my own > plugins,](https://github.com/Yilin-Yang/TypeVim/blob/4b75041aad7de2bf4121679a7631f51c1bc3b657/autoload/typevim/object.vim#L519) > at the very least, though the only other plugins I've found that do > this declare inside of the `l:` scope instead (e.g. > [LanguageClient-neovim](https://github.com/autozimu/LanguageClient-neovim/blob/8eba06bf6540915deeae7da9b239f0f918e20cd9/autoload/LanguageClient.vim#L515), > [ALE](https://github.com/w0rp/ale/blob/bf196ba17c9e261e4e3a9dba64260c6d0b2c8af9/autoload/ale.vim#L120)). It's a basic principle that arguments don't change. It helps a lot for debugging, since you can rely on the a: dict not to have changed somewhere. For optional arguments it's better to follow what most other languages do: func Foo(must_have, optional = value) Thus optional arguments always have a default value. That avoids having to check for whether they are present or not. It would also be possible to allow specifying the argument name in the call: func Foo(must_have, optional = value, also_optional = '') call Foo('must', also_optional = 'there') However, it has the disadvantage that changing the argument name in the function definition requires all calls to be changed, and one doesn't always know what calls use the name, which leads to mistakes. Just using positional arguments is simpler. One can pass a dictionary to have a bunch of optional arguments (as many built-in functions already do). -- Wi n0t trei a h0liday in Sweden thi yer? "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD /// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org /// — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3929 (comment)> -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to ***@***.*** ***@***.***>. For more options, visit https://groups.google.com/d/optout.

I wonder, should default argument expressions be parsed once at definition (like python) or every time at call (like ruby)? i.e., I assume this should be permitted: func Foo(must_have, optional = a:must_have) but of course, this should not: func Foo(may_have = a:second, second = 1)

brammool · 2019-02-12T21:28:55Z

I wonder, should default argument expressions be parsed once at definition (like python) or every time at call (like ruby)? i.e., I assume this should be permitted: func Foo(must_have, optional = a:must_have) but of course, this should not: func Foo(may_have = a:second, second = 1)

Python's way of doing this very often leads to mistakes. You cannot refer to other a: variables for the default value expression. It is evaluated before the a: dict is filled.

…

-- People who want to share their religious views with you almost never want you to share yours with them. /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

This fixes an issue with Vim 8.1.0887 and newer. When a change is landed, then revert back to canonical repo. Issue: chriskempson/base16-vim#197 Pull Request: chriskempson/base16-vim#198 More info: vim/vim#3929 Signed-off-by: Fletcher Nichol <fnichol@nichol.ca>

Issue: chriskempson/base16-vim#197 Pull Request: chriskempson/base16-vim#198 More info: vim/vim#3929

Problem: The a: dict is not immutable as documented. Solution: Make the a:dict immutable, add a test. (Ozaki Kiichi, Yasuhiro Matsumoto, closes vim/vim#3929) vim/vim@31b8160

fe0359761 Dependency updates c3ef7a77c Greenkeeper/default/husky 3.0.9 (#189) 2986331de fix typo (#185) 43986d71c Greenkeeper/default/husky 3.0.7 (#183) a9225db0f chore(package): update husky to version 3.0.4 (#179) af3c27193 Stop using doT for templating; dependency updates 4d28b36e8 Update husky in group default to the latest version 🚀 (#172) 904922988 Dependency updates 8bc66ee25 chore(package): update eslint to version 6.0.0 (#171) ef4a37a95 Adjust go highlighting (#170) 91128e181 Update dependencies 63b69b9f6 Fix alacritty cursor colors configuration. 7f36f83f1 Add specifics highlights for Go (#165) 782c195ac Update husky in group default to the latest version 🚀 (#163) d8d5f7fe7 Use higher-resolution color reference image. 7c30d3e4d Add color reference image. 723c616d4 Remove stray comment character ca2ae4d6d Added support for .tex files (#159) 36304e9b5 Don't peg package versions to reduce frequency of Greenkeeper updates 7b3229eec Don't peg verisons to reduce frequency of greenkeeper updates d32314d4a Update eslint in group default to the latest version 🚀 (#156) 9cd544f0f Update eslint in group default to the latest version 🚀 (#155) 07f6e3426 Update eslint in group default to the latest version 🚀 (#154) d1ada870e Update eslint in group default to the latest version 🚀 (#153) 205f57702 Update eslint in group default to the latest version 🚀 (#150) f65e71f25 Update eslint in group default to the latest version 🚀 (#149) a570035f4 README.md: Fix typo 04ba47974 Don't write to the "a:" dict per vim/vim#3929 . 2e817f0dd Update eslint in group default to the latest version 🚀 (#147) 4bf689976 Update eslint in group default to the latest version 🚀 (#146) 06bf8fa3b README.md: Fix a typo 682f86676 Add troubleshooting note about iTerm2. 598d37272 README.md: Better Greenkeeper badge placement 6b86d7861 Cleanup from initial Greenkeeper PR 4d342c91a Update dependencies to enable Greenkeeper 🌴 (#143) 438369790 Add :Termdebug highlight groups. fbf5af495 Tweak and document g:onedark_hide_endofbuffer 62bcd5ffb Add optional setting for end-of-buffer tildes (#142) a6325876f Fix usage of doT in build system 173a4df76 Oopsie. 5923008ce Hopefully fix CircleCI config dc8431c1a Add CircleCI 6d96889eb Added terminal statusbar highlight group (#138) 77c7b00eb Add a tweak for fish shell scripts (#135) 2a6155a25 Update README 7e03caaaf Add alacritty support (#132) 07ff25c34 Add terminal colors. cabf8e6df Fancy header image for README 6e219a958 Revise DiffChange/DiffText to be less ambiguous and less visually loud. a24854a55 README.md: Add drewtempelmeyer/palenight.vim to the "Relatives of onedark.vim" list 8ef6341ba Update build dependencies 4f0292533 Add plugin highlighting for easymotion/vim-easymotion. Closes #116. 8b5544e29 Enhance the implementation of onedark#extend_highlight and improve the README 62f84d27c Merge branch 'kristijanhusak-feature/extend-highlight-group' 88b7d3430 Nitpicky tweaks 3eb31282e Expose function for extending highlight group. d75941397 README.md: Fix formatting; add note about installation as a Vim 8 package. Closes #110 1552d3e8e In diff mode, for cursorline, use underline styling instead of background styling to improve readability. git-subtree-dir: bundle/onedark.vim git-subtree-split: fe035976117ba5c2481df3b2cad3bb0a8b045b9f

ichizok added 4 commits February 11, 2019 02:52

Add test

356f6e7

Fix: "a:" dict is not fixed

73902ff

Fix tests

e650eff

Import from mattn/vim@10b2a53

0f79e04

ichizok mentioned this pull request Feb 10, 2019

新しい a: スコープの変数を作成できてしまう vim-jp/issues#1231

Closed

mattn reviewed Feb 10, 2019

View reviewed changes

Add test for "a:000"

62c7d54

brammool closed this in 31b8160 Feb 10, 2019

ichizok deleted the fix/func_avar branch February 10, 2019 21:16

mojotx mentioned this pull request Feb 12, 2019

Onedark broken with Vim 8.1.888 joshdick/onedark.vim#148

Closed

2 tasks

joshdick added a commit to joshdick/onedark.vim that referenced this pull request Feb 12, 2019

Don't write to the "a:" dict per vim/vim#3929 .

04ba479

Closes #148.

gotgenes pushed a commit to gotgenes/dotfiles that referenced this pull request Mar 5, 2019

Switch to fork of base-16 until issue #197 is resolved

10e87be

Issue: chriskempson/base16-vim#197 Pull Request: chriskempson/base16-vim#198 More info: vim/vim#3929

nakp pushed a commit to nakp/dotfiles that referenced this pull request Mar 6, 2019

Switch to fork of base-16 until issue #197 is resolved

a7849ef

Issue: chriskempson/base16-vim#197 Pull Request: chriskempson/base16-vim#198 More info: vim/vim#3929

nakp pushed a commit to nakp/dotfiles that referenced this pull request Mar 6, 2019

Switch to fork of base-16 until issue #197 is resolved

03bb9e9

Issue: chriskempson/base16-vim#197 Pull Request: chriskempson/base16-vim#198 More info: vim/vim#3929

abdelhakeem mentioned this pull request Aug 20, 2019

[RDY] vim-patch:8.1.0888 neovim/neovim#10819

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix: can add the new value to "a:" dict#3929

Fix: can add the new value to "a:" dict#3929
ichizok wants to merge 5 commits intovim:masterfrom
ichizok:fix/func_avar

ichizok commented Feb 10, 2019

Uh oh!

mattn Feb 10, 2019

Uh oh!

ichizok Feb 10, 2019

Uh oh!

codecov-io commented Feb 10, 2019 •

edited

Loading

Uh oh!

Yilin-Yang commented Feb 12, 2019

Uh oh!

mattn commented Feb 12, 2019

Uh oh!

k-takata commented Feb 12, 2019

Uh oh!

brammool commented Feb 12, 2019 via email

Uh oh!

vim-ml commented Feb 12, 2019 via email

Uh oh!

brammool commented Feb 12, 2019 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

ichizok commented Feb 10, 2019

Uh oh!

mattn Feb 10, 2019

Choose a reason for hiding this comment

Uh oh!

ichizok Feb 10, 2019

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Feb 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Yilin-Yang commented Feb 12, 2019

Uh oh!

mattn commented Feb 12, 2019

Uh oh!

k-takata commented Feb 12, 2019

Uh oh!

brammool commented Feb 12, 2019 via email

Uh oh!

vim-ml commented Feb 12, 2019 via email

Uh oh!

brammool commented Feb 12, 2019 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov-io commented Feb 10, 2019 •

edited

Loading