luke-gru / riml

Riml is a subset of VimL with some nice added features. It compiles to plain VimL.
MIT License
224 stars 6 forks source link

Scope of variables becomes scriptlocal inside elseif #10

Closed dsawardekar closed 11 years ago

dsawardekar commented 11 years ago

@luke-gru This one is a bit tricky!

Consider the following code,

def get_foo(arg)
  if arg == 'a'
    return 'A'
  elseif arg == 'b'
    return 'B'
  else
    return 'Unknown'
  end
end

This compiled to the following viml.

function! s:get_foo(arg)
  if a:arg ==# 'a'
    return 'A'
  elseif s:arg ==# 'b'
    return 'B'
  else
    return 'Unknown'
  endif
endfunction

Here arg is an argument, but in the elseif condition it becomes scriptlocal, s:arg, it should be a:arg. Further this also happens with local variables.

def get_bar()
  my_var = 'foo'

  if my_var == 'a'
    return 'A'
  elseif my_var == 'b'
    return 'B'
  else
    return 'Unknown'
  end
end

Compiles to,

function! s:get_bar()
  let my_var = 'foo'
  if my_var ==# 'a'
    return 'A'
  elseif s:my_var ==# 'b'
    return 'B'
  else
    return 'Unknown'
  endif
endfunction

Here s:my_var should be just my_var.

Both these functions result in undefined variable errors when the functions are called. This bug only appears in the elseif, just if-else is fine.

My current workaround is to explicitly scope all variables used inside elseif with their scope. Eg:- l:my_var and a:args

luke-gru commented 11 years ago

Thanks for the detailed issue report! This is fixed in the new version I just released, 0.3.0. It had to do with not putting the ElseifNode's condition expression in the children of the node, woops! :smile:

This new release also contains the default class scoping to 's:' (class_scopes branch), along with another branch I merged in called private_funcs, that makes private functions (defined with def, not defm) a lot better to work with. You can check an example of a private function in the CHANGELOG. I'll be adding docs to the README as well shortly.

Oh, and as for the release process, I'll be merging in your PR in a bit.

Thanks again!

dsawardekar commented 11 years ago

Awesome! I upgraded Speckle to 0.3.0. Everything went smoothly. I needed to use g: for code that I'm evaling. And I removed some of the `l:' workarounds in elseifs. All those bugs fixes worked great. :)

I'm pleased to report that the switch to default s: scope for classes did not cause any trouble. I didn't even need change any of my specs! Also thanks for fixing the splats bug. I was going to report that too, but didn't want to impose too much. :)

The private scoped def looks neat. Prefixing <SID> should make it pretty safe. Though I'm sure some Vimscript Jedi master is going to figure a way around it!