Better Lib APIs

[mitnk]

via #3

We need to rename line_to_tokens to cmd_to_tokens.

it would be also cool to have it indicate if the command is complete e.g. the return value of line_to_tokens("foo |") should make clear that this isn’t a runnable command.

[mitnk]

@flying-sheep for line_to_tokens(), I'd like to keep what it does now to keep it simple. The caller should make sure the input is valid for line_to_tokens().

How about provide another API to indicate whether one command line is valid or not?

[flying-sheep]

sure, if that’s how you want it!

it’ll mean duplicate tokenization though.

[mitnk]

Actually I don't need to touch any logics inside cmd_to_tokens(), and just add a new one: is_valid_input.

Please see it here: https://docs.rs/cicada/0.7.2/cicada/

[flying-sheep]

Actually I don't need to touch any logics inside cmd_to_tokens(), and just add a new one

yes! but what i meant is that if you want to check if it’s complete and then tokenize it, you end up parsing it twice. (once for validation, again for tokenization).

but that’s probably premature optimization

is_valid_input

“invalid” can mean two things:

1. syntax error
2. incomplete command line that would be valid if a closing brace or a command to pipe into existed.

[mitnk]

“invalid” can mean two things

Is there any cases you wanted not consistent with current is_valid_input()?

[flying-sheep]

yes: in an advanced prompt, once you hit return, one of the following things happens:

1. it runs the command
2. it jumps to the next line since a string or brace is unclosed, or a pipe or && has no right operand.
3. it displays a syntax error.

if is_valid_input is the only thing you can do before trying to run it, you can’t distinguish between 2 and 3.

i think i’d have designed cmd_to_tokens to return a Result, with an error enum that allows to distinguish between several kinds of syntax errors. (and maybe a convenience method that can be used to check if the error is of an “incomplete input” kind, or a non-salvagable kind)

[mitnk]

Simplify a bit since 0.8.9 - only left run() and cmd_to_tokens() in the API list - https://docs.rs/cicada/0.8.9/cicada/#functions

I'm closing this issue, please open new ones if any further discuss.