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

Use os:cmd instead of Erlang ports #22

Closed
gootik opened this issue Sep 20, 2020 · 8 comments · Fixed by #23
Closed

Use os:cmd instead of Erlang ports #22

gootik opened this issue Sep 20, 2020 · 8 comments · Fixed by #23

Comments

@gootik
Copy link
Owner

gootik commented Sep 20, 2020

os:cmd handles many edge cases and in general is better.

@gootik
Copy link
Owner Author

gootik commented Sep 20, 2020

Would also fix #16

@paulo-ferraz-oliveira
Copy link
Contributor

I don't think the current implementation supports it, but I'd like to see commands expressed so:

{check, ["make xref", "make dialyzer"]} % list of strings

and also

{test, "make test"} % single string

in which case, a "quickfix" for #16 would be to use the new syntax (list of string).

@gootik
Copy link
Owner Author

gootik commented Sep 24, 2020

@paulo-ferraz-oliveira I like the first suggestion. but I still think we should change to os:cmd.

@paulo-ferraz-oliveira
Copy link
Contributor

I still think we should change to os:cmd.

Oh, I was intending on using it 😄. When I wrote "I don't think the current implementation supports it" I was referring to what I was going to expose below it.

So:

  1. use os:cmd,
  2. implement a command syntax like:
{check, ["make xref", "make dialyzer"]} % list of strings

while not breaking backward compatibility.

@gootik
Copy link
Owner Author

gootik commented Sep 24, 2020

yeah that's perfect

@paulo-ferraz-oliveira
Copy link
Contributor

With the previous implementation you could always recover the return code from the command, which won't be possible with os:cmd it seems.

Is this acceptable?

@paulo-ferraz-oliveira
Copy link
Contributor

@paulo-ferraz-oliveira
Copy link
Contributor

I ended up not implementing this syntax

{check, ["make xref", "make dialyzer"]} % list of strings

as several questions arose that would make its UX difficult.
If its asked for, in the future, we could consider it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants