config: Call hotplug hooks in more places #1233
Conversation
| local cf = io.open(name) | ||
| if not cf then | ||
| return false | ||
| else | ||
| cf:close() | ||
| return true | ||
| end |
There was a problem hiding this comment.
You could also use fs.stat(filename, "type") == "reg" for this, no?
There was a problem hiding this comment.
Mnyaaaa.. good question. I looked at this myself. Initially the function was just moving around existing code. When I looked at the general consensus on how to do this in Lua, I found the following:
"Using plain Lua, the best you can do is see if a file can be opened for read, as per LHF."
-- https://stackoverflow.com/questions/4990990/check-if-a-file-exists-with-lua
I believe "LHF" here refers to Luiz Henrique de Figueiredo, the creator of Lua.
So this is both in line with the Lua way of doing things, and it's what the existing code already does so I left it as is. That said, I'm certainly open to changing it to be more POSIXey but I was trying to do things in a Lua way because my Lua experience is very limited.
There was a problem hiding this comment.
Good point. Both seems fine to me.
There was a problem hiding this comment.
I suggest to switch to @pony1k suggested version as it is more compact and we already depend on those libraries
| local cf = io.open(name) | ||
| if not cf then | ||
| return false | ||
| else | ||
| cf:close() | ||
| return true | ||
| end |
There was a problem hiding this comment.
I suggest to switch to @pony1k suggested version as it is more compact and we already depend on those libraries
| end | ||
| end | ||
|
|
||
| function config.execute_hooks(action) |
There was a problem hiding this comment.
This function or something very similar feels like to be used/needed in more place in LiMe code, let's try to avoid duplication
There was a problem hiding this comment.
Are you suggesting the function should be moved? If so, where would you suggest it be moved to?
There was a problem hiding this comment.
yes, this function is relevant not just to the config module, but to any module that might want to run some hook (all). In fact I am surprised that something similar doesn't already exists, have you double checked?
There was a problem hiding this comment.
have you double checked?
I haven't single checked :-) I'm not sure what your expectation is here. What do you expect me to have checked? Bearing in mind I don't know either the codebase or Lua well at all. Can you tell me where you would expect me to check?
rah2501
force-pushed
the
config-hotplug
branch
2 times, most recently
from
e945df2 to
f92270a
Compare
Also add an ACTION environment variable containing the same value as passed as a command-line argument. Some scripts presume they will only be called in the one existing place so they need to be updated also.
rah2501
force-pushed
the
config-hotplug
branch
from
f92270a to
2cb1fbf
Compare
4 participants
Also add an ACTION environment variable containing the same value as passed as a command-line argument.
Some scripts presume they will only be called in the one existing place so they need to be updated also.