Skip to content

Change time from builtin to shell keyword.

Michael Aaron Murphy requested to merge chrisvittal:time-command into master

Created by: chrisvittal

Problem: The time command could not accept shell commands like builtins or for loops. #490 (closed)

Solution: Refactored time from a builtin to a keyword. That is, create a Statement::Time and handle the time logic in the execution of Statement::Time variants, rather than as a builtin function.

Changes introduced by this pull request:

  • A Time variant of Statement a boxed statement member.
  • Support for parsing the time keyword in parser/statement/parse.rs by recursing on the rest of the input to the parse function when time is encountered.
  • A port of the logic from the old builtin into the execute_toplevel impl for Shell, where instead of using the error handling from Command, the time statement is executed through a recursive call to execute_toplevel.
  • The removal of all references to the old time builtin.

Drawbacks: The loss of some friendliness. The time -h invocation no longer works.

TODOs:

  • Add tests.
  • Time currently cannot handle multiline statements like quotes being spread over multiple lines. Fix this.
  • Remove builtins/time.rs file
  • Edit: 2017-08-23 Implemented at current, a time statement inside of a loop supresses all output. Fix this.

Fixes: #490 (closed)

State: WIP

Merge request reports