hush-shell / hush

Hush is a unix shell based on the Lua programming language
MIT License
658 stars 23 forks source link

Question: std.catch with parameters #28

Closed JakubWagner1019 closed 2 years ago

JakubWagner1019 commented 2 years ago

I'm trying to handle integer parsing error and the best I came up is this:

function to_int(x)
    return std.catch(
        function()
            return std.int(x)
        end
    )
end

Seems a little awkward to me. Is there a better way to pass arguments to function in std.catch?

gahag commented 2 years ago

Hey, thanks for reporting the issue. After giving it a thought, I think the current behavior of std.int (and std.float) for parsing strings was a big oversight on my part. These should return an error instead of panicking. I'll try to find some time today to fix that.

JakubWagner1019 commented 2 years ago

These should return an error instead of panicking

100% agree. When first trying out these functions intuitively I expected that to be the case. I was a little surprised you went for panic instead of error here. I think every function that we can prevent from failing by accurate check before the function call should panic. If we can't check before executing, then return an error. While this answers the problem I've had, it doesn't directly address the question I've asked. I can't think of any now, but there may be similar issues in the future to what I've had. I mean the scenario when someone wants to wrap a panicking function with std.catch (maybe while using some faulty external library). I noticed test cases for std.catch only cover no args functions. I'm actually not sure whether implementing it will be the right decision as such feature may encourage people to use std.catch more while as per tutorial it was not advised to go this way. I see however, two possibilities in which this may be implemented. First, make std.catch to produce a wrapper to any function which will have the same number of args as the wrapped function. f.e.

let safe_int = std.catch(std.int)
let integer = safe_int("4")

or shorter version let integer = std.catch(std.int)("4")

Second option, make std.catch use 'varargs' for parameters. let integer = std.catch(std.int, "4")

What do you think?

JakubWagner1019 commented 2 years ago

I think the first implementation makes much more sense.

gahag commented 2 years ago

Hush's std.catch is largely inspired in Lua's pcall. As you mentioned, it should be used sparingly, and only in very specific scenarios. Therefore, I don't it's worth to make it ergonomic for error handling. If you really need to use it in that way, say for working around a faulty external library, you can do it on call site.

Instead of

let result = faulty_lib.method(arg1, arg2)

You can do

let result = std.catch(function () faulty_lib.method(arg1, arg2) end)

And it'll work as expected.

By the way, I'm considering changing the function keyword to fun, so that examples like this will be even less verbose.

Let me know what you think :)