Can't tell return value of programs with staticExec #1994

Closed
def- opened this Issue Jan 22, 2015 · 9 comments

Projects

None yet

6 participants

@def-
Member
def- commented Jan 22, 2015

Using staticExec the return value of the executed program can't be checked. Even the manual's example of git rev-parse HEAD can fail if you're not inside a git repo, which you can't catch nicely. I think staticExec should either return an empty string if the return value isn't 0, or have a way to get back the return value as well as stdout.

@hildjj
hildjj commented Jan 26, 2015

Throw an exception if the return code is non-zero?

@Varriount
Contributor

It's not quite as simple as that (besides, exceptions should be used for unexpected, exceptional cases). Different platforms have different ideas on what a error return code is.

@kirbyfan64
Contributor

There are three ways I've seen:

  1. Return a tuple.
  2. Return a custom object. This is what Python does.
  3. Use var parameters. This seems very ugly to me.

Personally, I think 2 would be the best, since platform-specific error codes might not matter; it could just have a succeeded boolean attribute.

@Araq
Member
Araq commented Jan 27, 2015

2 is by far the hardest to implement and 1 affects backwards compatibility. What should be done is that the proc grows an "onErrorRaise= true" flag and then an OSError exception is raised. If set to false the empty string is returned instead.

@dom96
Member
dom96 commented Jan 27, 2015

Wouldn't it make sense to force users to check the return code of the process? Therefore breaking backwards compatibility should be acceptable in this case.

@Araq
Member
Araq commented Jan 27, 2015

@dom96 Returning a tuple is quite hard to do in the VM though.

@dom96
Member
dom96 commented Jan 27, 2015

@Araq Why is it hard?

@Araq
Member
Araq commented Mar 12, 2015

Now at least it produces "" instead of crashing, I think.

@dom96
Member
dom96 commented Mar 12, 2015

That sounds like it will lead to some ambiguities.

@Araq Araq added Feature and removed High Priority labels Mar 22, 2015
@flyx flyx added a commit to flyx/Nim that referenced this issue Oct 9, 2016
@flyx flyx Fixes #1994 0f5a411
@Araq Araq closed this in 9d48826 Jan 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment