elves / upgrade-scripts-for-0.17

Upgrade Elvish scripts for 0.17
BSD 2-Clause "Simplified" License
4 stars 2 forks source link

Errors would be improved by replacing `[stdin]` with the file name if it is known #1

Closed krader1961 closed 2 years ago

krader1961 commented 2 years ago

I ran upgrade-scripts-for-0.17 -w **.elv in my ~/.config/elvish directory. It reported a bunch of errors like this one:

Multiple parse errors:
  should be variable name
    [stdin], line 174:
        &su= $&segment_su

It wasn't hard to find the offending script but it would be nice if the error message reported the pathname rather than [stdin]. In this case the error(s) were due to lib/github.com/muesli/elvish-libs/theme/muesli.elv that I installed long, long, ago but never used after trying it once. It might also be better to simply stop after the first parse error since a parse error means that any further effort to convert the script is unlikely to yield useful results and likely to result in a lot of additional parse errors.

krader1961 commented 2 years ago

I also got this error: open lib/github.com/xiaq/edit.elv: is a directory. The error is self explanatory, and due to an unusual name for an Elvish package (see https://github.com/xiaq/edit.elv). It's probably not worth complicating the implementation to suppress the message in this case but I'm mentioning it because any error is likely to surprise a user.

krader1961 commented 2 years ago

I also got this error in response to running upgrade-scripts-for-0.17 -w **.elv. It was not at all obvious where it originated:

parse error: should be ')'
[stdin], line 47:   }

I tracked it down to my lib/semver.elv module I was working on before realizing @zzamboni had already solved the semver problem and abandoned work on my implementation. I did have a syntax error. Apparently I was in the middle of augmenting the logic and neglected to add a right-paren to the (and... expression in this block of code at the time I abandoned work on my module:

    for i (range $min-len) {
        if (and (re:match '^\d+$' $l1[$i]) (re:match '^\d+$' $l2[$i]) {
            # Both prerelease components are numbers so compare them numerically.
        }
    }
xiaq commented 2 years ago

I believe the issues are resolved. Let me know if I missed anything.