implement (naive) `exec` builtin command #36

KeenS · 2021-12-06T09:01:49Z

This P-R partially implements exec command. It only supports exec cat maggie syntax and doesn't exec 3< readfile.

nuta

Thank you for this PR, @KeenS! Generally looks good to me. Can you take a look at some nitpicks from me.

nuta · 2021-12-13T12:14:17Z

src/builtins/exec.rs

+        };
+
+        match nix::unistd::execvp(&command, &args) {
+            Ok(never) => match never {},


Nit: I believe we can assume execvp won't return a success value.

Suggested change

Ok(never) => match never {},

Ok(_) => unreachable!(),

Exactly. Because so, I'm matching against Infallible, which is stronger assertion than ignoring value and calling unreachable .

Sounds better! I have never heard of never. Thank you for explanation 😃

src/builtins/exec.rs

Co-authored-by: Seiya Nuta <nuta@seiya.me>

nuta · 2021-12-15T11:12:15Z

Thank you again @KeenS!

implement (naive) exec builtin command

56e353c

nuta reviewed Dec 13, 2021

View reviewed changes

KeenS and others added 2 commits December 14, 2021 11:03

Update src/builtins/exec.rs

bafe870

Co-authored-by: Seiya Nuta <nuta@seiya.me>

Update src/builtins/exec.rs

b37c18b

Co-authored-by: Seiya Nuta <nuta@seiya.me>

nuta approved these changes Dec 15, 2021

View reviewed changes

nuta merged commit 4e90833 into nuta:main Dec 15, 2021

Provide feedback