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.

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

Focused_node and empty dir #417

Closed
igorepst opened this issue Nov 18, 2021 · 11 comments · Fixed by #419
Closed

Focused_node and empty dir #417

igorepst opened this issue Nov 18, 2021 · 11 comments · Fixed by #419

Comments

@igorepst
Copy link
Contributor

Hi @sayanarijit.
It seems there are two separate issues that may be reproduced by the same way, but, perhaps, this is just something I don't understand.
Given the following init.lua and xplr 0.16.4:

version = '0.16.0'
xplr.config.modes.builtin.default.key_bindings.on_key['f4'] = { help = 'edit file', messages = { { CallLua = 'custom.edit_file' } } }
xplr.fn.custom.edit_file = function(a)
    os.execute('echo ' .. (a.focused_node and 'focused is NOT nil' or 'focused is nil') .. '>>/tmp/fn 2>&1')
    os.execute('echo ' .. a.focused_node.pwd .. '>>/tmp/fn 2>&1')
end

If you go to an empty directory and press 'F4', the following happens:

  1. /tmp/fn is created with a string focused is NOT nil, although as the directory is empty, I would say that the focused_node should be nil. Documentation, at least from my point of view, suggests this is wrong, describing focused_node as Type: nullable Node. And at the same time the second os.execute throws exception that may be seen in logs
 ERROR   runtime error: [string "init"]:5: attempt to index field 'focused_node' (a userdata value)
stack traceback:
        [string "init"]:5: in function <[string "init"]:3>

suggesting that it is actually not a table, but a userdata.
2. At the same time the above exception produces log string that is actually cannot be seen in logs using the above almost default configuration. I mean I see this:
image
Here, probably, the first line of the error should be seen

Thanks!

@sayanarijit
Copy link
Owner

sayanarijit commented Nov 19, 2021

  1. I'd use this to print the table and io.read() to pause the execution after printing the table. This allows me to see what really happens.
  2. There could be a new line character in the log which caused the empty line. We can probably strip it, but then also, we won't get much info if the log has multiple lines and the last line has only 1 or 2 words. We'll need to type :l to see the whole log.

@igorepst
Copy link
Contributor Author

Regarding logs, understood. I did the same thing, just thought it would be better to show the first line of each log entry and not the last one, if the view cannot present it fully.
Now, regarding userdata. The current situation causes issues, for ex., in your plugin alacritty.xplr, as the right line here should be

        if args.send_focus and type(app.focused_node) == 'table' then
...

otherwise, the same exception attempt to index field 'focused_node' (a userdata value) is thrown.
Here I should add that even though I have 17 years of professional experience in programming and I understand intuitively what the Rust code does when I read parts of it from your repo, it is tough as I don't know Rust at all and in the near future I won't find a time to dive deeper. So I think there are 2 options:

  1. Potentially break backward compatibility and return nil here, checking, perhaps, that focused_node.pwd is not null.
  2. Document it, either for focused_node specifically or for Node overall if it may happen in other places (I may do it and open a PR)

Thank you :)

@sayanarijit
Copy link
Owner

Ah thanks for the detailed explanation. This is definitely an issue. After some research, I found Rust's None serialize into Lua null, which represents an uninitialized variable name. It can be configured to be serialized into nil by setting serialize_none_to_null to false, but it's not the default for some reason.

However, I'm not sure why we get userdata instead of the variable null which should actually point to the value nil.

@sayanarijit
Copy link
Owner

sayanarijit commented Nov 19, 2021

Let's play around a bit.

  1. Create a file named x.lua with the following code.
xplr.config.modes.builtin.default.key_bindings.on_key["#"] = {
  help = "edit file",
  messages = { { CallLua = "custom.edit_file" } },
}

xplr.fn.custom.edit_file = function(a)
  print(null)
  print(nil)
  print(a.unknown_field)
  print(a.focused_node)
  io.read()
end
  1. Open xplr with xplr -C x.lua
  2. Go to an empty directory, so that a.focused_node is None.
  3. Press #

We get the following result:

nil
nil
nil
userdata: NULL

I get userdata: NULL when I tried to pretty-print it.

I wasn't able to find any definition of userdata: NULL, so, I think it's some custom alternative to null which is different from nil in the sense that it isn't undefined (e.g. a.unknown_field). I guess that could be the intention behind serialize_none_to_null being set to true by default. Pinging @khvzak to confirm that it's indeed the case.

Anyway, I think for our use case, it makes sense to set serialize_none_to_null to false and use nil instead.

@igorepst
Copy link
Contributor Author

As far as I understood the linked StackOverflow answer, from Lua's point of view, there is nothing special in null, it is just a regular valid variable name. As Lua doesn't require the variable to be declared before usage,
v=null is the same as v=nil if and only if null==nil, but
null='bla'; v=null will lead to v='bla'

@igorepst
Copy link
Contributor Author

See pub trait LuaSerdeExt<'lua> at https://github.com/khvzak/mlua/blob/f0f5a8a0af62aff8007b20d58ef8f92b9ba8bc5d/src/serde/mod.rs#L17
Sorry for the formatting, I am writing from phone.
This expains the userdata

@sayanarijit
Copy link
Owner

As far as I understood the linked StackOverflow answer, from Lua's point of view, there is nothing special in null, it is just a regular valid variable name. As Lua doesn't require the variable to be declared before usage, v=null is the same as v=nil if and only if null==nil, but null='bla'; v=null will lead to v='bla'

Yes, that's what I mentioned, there's no concept of null in Lua, But maybe the serialization library needed an alternative to None and that's why a custom userdata: NONE was implemented. I'm not sure though. But yes, it's easy to fix. We only need to set an option.

@sayanarijit
Copy link
Owner

But yes, it's easy to fix. We only need to set an option.

Probably not.

@sayanarijit
Copy link
Owner

Ok fixed. I guessed right.

nil
nil
nil
nil

sayanarijit added a commit that referenced this issue Nov 19, 2021
sayanarijit added a commit that referenced this issue Nov 19, 2021
@khvzak
Copy link

khvzak commented Nov 19, 2021

null (lightuserdata 0x0 pointer) is an alternative to nil to keep fields in a table after serializing Rust structs with optional fields to Lua value.

Same as null in lua-cjson

Lua CJSON decodes JSON null as a Lua lightuserdata NULL pointer. cjson.null is provided for comparison.

It used by default to get the same result when applying lua.from_value(lua.to_value(val)?)? transformation.

@sayanarijit
Copy link
Owner

Got it. Thanks.

sayanarijit added a commit that referenced this issue Nov 19, 2021
sayanarijit added a commit that referenced this issue Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants