elves / elvish

Powerful scripting language & versatile interactive shell
https://elv.sh/
BSD 2-Clause "Simplified" License
5.52k stars 296 forks source link

Syntax errors in docs code examples #1780

Closed Duncan3142 closed 3 months ago

Duncan3142 commented 3 months ago

What happened, and what did you expect to happen?

try/else

The documentation for the else block of the try structure contains a syntax error.

The following example is given

~> try { nop } else { echo well }
well

However, the following code fails

~> try { which notonpath } else { echo found }
Compilation error: try must be followed by a catch block or a finally block

?(...)

The documentation for exception capture contains a syntax error.

The following example is given

~> var output = (var error = ?(commands-that-may-fail))

However the following code fails

~> var output = (var error = ?(which notonpath))
/usr/bin/which: no notonpath in (/home/dg/.volta/bin:/home/dg/.volta/bin:/home/dg/.volta/bin:/usr/bin:/home/dg/.volta/bin:/usr/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin)
Exception: arity mismatch: assignment right-hand-side must be 1 value, but is 0 values
  [tty 33]:1:1-46: var output = (var error = ?(which notonpath))

I believe the intended example should be

var status = ?( var output = (which notonpath))

Output of "elvish -version"

0.21.0-dev.0.20240327140325-fa43bb31209d+official

Code of Conduct

krader1961 commented 3 months ago

The first documentation problem involving try... else... was noted in #1734, but it's great that a new user has noticed the same issue. That new users have noticed the same issue makes it even more important to improve that aspect of the documentation.

Duncan3142 commented 3 months ago

In that case, I'll raise a PR to fix the try... else... example in the docs.

Perhaps it would have been better for me to create two separate issues, as I feel the ?(...) docs example and explanation are fundamentally at odds with each other.

The explanation

Note: Exception captures do not affect the output of the code chunk. You can combine output capture and exception capture:

doesn't align with the code example

var output = (var error = ?(commands-that-may-fail))

Would it be better for me to close this issue, create a PR to address the existing issue #1734, then create a separate issue for discussion of the exception capture docs?

Duncan3142 commented 3 months ago

Here is a PR to address #1734

Duncan3142 commented 3 months ago

As an observation, the syntax for try/else/finally is a bit confusing as a new user.

The syntax implies the else happens instead of the try

Also, is there a need for such a construct?

For example, instead of

try {
  cat log-file > /tmp/log-snapshot
} else {
  aws s3api put-object --bucket logs --key logs/latest --body /tmp/log-snapshot
} finally { 
  rm -f /tmp/log-snapshot
}

would it not be sufficient to simply do

try {
  cat log-file > /tmp/log-snapshot
  aws s3api put-object --bucket logs --key logs/latest --body /tmp/log-snapshot
} finally { 
  rm -f /tmp/log-snapshot
}

If there is a valid use case for try/else/finally I'd be happy to add it to the above PR

If try/else/finally doesn't make sense, perhaps it would be better to make else conditional on the presence of catch?

hanche commented 3 months ago

As far as I can tell, an else block without both a catch block and a finally block has no purpose. If there is no catch, you can merge the try and else blocks, and if there is no finally, you just put the content of the else after the try statement. (And if there is neither a catch nor a finally block, the trystatement is a syntax error anyhow, so the point is moot. But that raises the question whether an else block without both catch and finally should be a syntax error too.)

hanche commented 3 months ago

I forgot to address this one:

The syntax implies the else happens instead of the try

As in if/else, you mean? The intent is that the else happens instead of the catch when there is no error, but if there is no catch block, it does indeed get a bit confusing. That's another argument for actually restricting the syntax to avoid this problem.

Duncan3142 commented 3 months ago

Thanks for clarifying

@xiaq said the same thing

https://github.com/elves/elvish/pull/1782#issuecomment-2030433952

An else doesn't make sense in the absence of a catch

Duncan3142 commented 3 months ago

As far as I can tell, an else block without both a catch block and a finally block has no purpose. If there is no catch, you can merge the try and else blocks, and if there is no finally, you just put the content of the else after the try statement. (And if there is neither a catch nor a finally block, the trystatement is a syntax error anyhow, so the point is moot. But that raises the question whether an else block without both catch and finally should be a syntax error too.)

In version 0.21.0 an else without either a catch or finally is an error, which is what prompted the issue. =)

Thanks for the clarification though. I'm super excited about the future of Elvish