This also replaces the slightly awkward "arguments here" reason with
"argument count" as the "what" for a typical errs.ArityMismatch
exception. It also reformats most of the constructors so that the "what"
is on the same line. This makes `grep errs.ArityMismatch **.go` more
useful as a result.
This change makes feeding output to commands which handle NUL terminated
"lines" (e.g., `fzf -read0` or `xargs -0`) extremely fast compared to
using an explicit Elvish loop that does `print $val"\x00"`. Similarly for
handling input from commands that produce NUL terminated "lines" (e.g.,
`find . -print0`) compared to an Elvish loop using `read-upto "\x00"`.
Resolves#1070
Related #1053
It is in theory better implemented as a StructMap because it is a transparent
data type. However, the reflection-based algorithm for StructMap will create a
"kind" field for it, and we don't want to remove the custom kind of Pipe yet, so
this has to be a PseudoStructMap now.
In light of the preceding change to address staticcheck lint warnings I
ran this:
grep 'errors\.New("' **.go |
sed -e 's/^.*errors\.New("\([^"]*\)".*/\1/' |
sort | uniq -c | sort -n
Which caused me to notice two errors associated with the builtin `sleep`
command have multiple definitions. It is questionable whether the negative
duration error is justified. I think it should be replaced by the invalid
duration error but decided not to do that in order to limit the scope of
this change.
Fix two issues found by `staticcheck -tests=false ./...`:
pkg/eval/go_fn.go:155:5: var errNoOptions is unused (U1000)
pkg/persistent/hashmap/hashmap.go:321:2: only the first constant in this group has an explicit type (SA9004)
I noticed that testutil.MustPipe was not covered by any unit tests. This
converts the handful of places that should use it to do so. This changes
the coverage of pkg/testutil/must.go from 61.5% to 73.1%. There isn't
any way to increase that further without explicitly testing the panic
paths.
This implements `&dedup` and `&newest-first` options for
`edit:command-history`. This makes it noticably cheaper to feed unique
command history into external commands like `fzf`.
Related #1053Fixes#568
Rather than having specialized commands make a `file:pipe` object
indexable so we can use the generic `file:close` command. This does not
address existing problems; such as builtins not failing when writing to
a `file:pipe` object if the read-end is closed.
Related #1316
Creating symlinks requires a special permission on Windows, which the
use may not have.
Also remove the support for symlinks in testutil.ApplyDir. Because of
this issue on Windows, any test that requires creating symlinks needs to
be broken out to an individual test case, meaning that symlinks can't be
created alongside other files, so there is less benefit of keeping it
inside the same Dir structure.
The `edit:after-command` hooks are called with a single argument:
a pseudo-map with these keys:
"command": the command line that was run
"duration": the execution duration in seconds
"error": any error that occurred ($nil if no error occurred)
The `edit:command-duration` variable is the elapsed seconds (as a
float64) of the most recently run interactive command.
Resolves#1029
* Remove `MatchesRegexp` from eval/vals since it is not part of the
Elvish value protocol.
* Simplify implementation of value matching in `eval/evaltest`.
* Documentation wording tweaks.
These tests exercise the arity checking logic, which is implemented
generically for every builtin function and does not need to be tested
individually.
Prclose and pwclose with Deprecation marked in specific file. Give
feedback regarding the test cases.
File module now has prclose and pwclose
Wrote test cases for prclose and pwclose, need better test cases
Deprecation for prclose and pwclose marked in respective files.
Using a read-only variable as the target of an `except` clause should
highlight just the var name rather than the entire `try...except...`
statement.
Resolves#1258
When attempting to update a read-only var return an error struct rather
than a simple string (i.e., a Go `error` type). This makes it possible
to include the var name in the error message. This builds on commit
a33ecb2d that highlights the offending var name in the stack trace but
does not include the var name in the error message. With this change the
error message includes the offending var name.
Related #255
The "-count" flag causes the test functions to be invoked multiple times. Since
some tests mutate their test data or some global state, they will fail when
invoked with a count larger than 1.
This commit changes *some* of such tests to either avoid mutating the test data,
or resetting the global state. Some such tests still remain and will be fixed later.
Previously, to avoid showing deprecation warnings for the next release when the
user is running on HEAD, a boolean CLI flag -show-deprecations is introduced,
and is set to false in the master branch. The idea is that release branches will
have this default to true, so people running released versions will see
deprecations.
However, this means that people running on HEAD will never see any deprecations
unless they use this CLI flag, which is not ideal. This commit replaces the flag
bool -show-deprecations with a numerical -deprecation-level flag, which requests
deprecations that are active as of release 0.X to be shown. The default value of
this flag will be the minor version number of the *last* release, so that people
running HEAD will see as many deprecation warnings as people running the last
release would. This number will be bumped just before releases.
* Introduce the `path:` module
This is based on https://github.com/elves/elvish/pull/1084 by @kolbycrouch
submitted five months ago. It addresses all of the feedback on that
change and includes other documentation and unit test improvements. It
also includes a couple of extensions to the original P.R., such as a
`path:is-abs` command.
I decided to resurrect that change because I want better support for
filesystem path manipulation so that users can replace non-portable
external commands such as `realpath` and `find` with Elvish builtins. This
is a baby step towards that goal.
Related #849
* Add a `path:is-regular` command
This adds a `path:is-regular` command. This is for symmetry with the
`path:is-dir` command and the glob `[type:regular]` modifier.
It also adds support for symlinks in the `testutil.Applydir` function
and change the path unit test to use it.
* Rename path:real to path:eval-symlinks
There is more work to be done but this should address 99.9999% of the
expected uses of a `printf` builtin. Some corner cases which don't have
a POSIX analog, such as support for Elvish bool types will be implemented
in future changes.
Resolves#1132
The implementation of useOp has not been updated correctly to accomodate the
fact that the namespaces are no longer modified in place.
This fixes#1212.
This commit replaces scopeOp, the only remaining place that mutates *Ns in
place, with nsOp, which performs copy-on-write for *Ns.
As a result, the "eval" command no longer mutates the passed namespace. The
default namespace it uses is also changed to match the default of "-source" (an
amalgamated namespace from the local and upvalue scopes), making "-source $file"
equivalent to "eval (slurp <$file)", and formally deprecated.
Another result is that (*Evaler).Eval can now guard the mutation of the global
namespace with the mutex, making it concurrency-safe to execute multiple sources
that touch the global namespace.
This fixes#1137.
Also change the variable name used to keep the Exception returned from "err" to
"exc".
This uncovers several error scenarios that were not returning Exception, and
would result in the absense of stack traces when such errors occur.
This change is a preparation step for refining all *Op types to return Exception
as the error.
Keeping Exception as a struct type will make such a change error-prone, since
a (*Exception)(nil) != error(nil), so if an *Op returns a nil *Exception, it
is not nil if the return value is stored in an error-typed variable.
Doing something like the following is likely to result in too many open
files (assuming `ulimit -n` == 256) resulting in a panic:
> fn fact [n]{ if (== $n 0) { put 1 } else { put (* $n (fact (- $n 1))) } }
> fact 60
panic: interface conversion: error is *os.SyscallError, not
*eval.Exception
goroutine 24161 [running]:
github.com/elves/elvish/pkg/eval.(*pipelineOp).exec.func1(0x152a5a0,
0xc000dca210, 0xc000a44e70, 0xc00057b540, 0xc001030590, 0x203000)
github.com/elves/elvish/pkg/eval/compile_effect.go:153 +0x152
created by github.com/elves/elvish/pkg/eval.(*pipelineOp).exec
github.com/elves/elvish/pkg/eval/compile_effect.go:149 +0x225
Exception: elvish exited with 2
Fixes#1208
- Move NewEnvListVar to pkg/eval/vars.
- De-export GlobPattern, GlobFlag and ExternalCmd.
- Merge editor.go and chdir.go into eval.go, value_helper.go into compile_value.go.
- Remove eval_internal_test.go and replace it with a new test for $pid.
This change makes Ns immutable from the exposed API. Internally there is exactly
one place that still mutates Ns, in scopeOp; this will be addressed later.
- Make Evaler mostly thread-safe. The only remaining thread-unsafe part is the
modules field, which is more tricky than other fields.
- Remove the state and evalerScopes type, and move their fields into Evaler.
- Expose valuePrefix via a get method, and change PortsFromFiles to take the
prefix instead of a *Evaler. Also expose a PortsFromStdFiles.
- Make Evaler a normal field of Frame, instead of an embedded field. This makes
access to global states more explicit.
The Evaler keeps global states and needs to be accessed concurrently. Mutations
to global states have fairly low throughput, so it makes sense to use a single
mutex.
On the other hand, the compiler is always used on a single thread, so it does
not need any mutex protection, so there is no need to put the mutex inside
deprecationRegistry.
- Remove the Op type; it is no longer used by any code outside the eval package
and its use within the package is limited.
- Replace (*Evaler).execOp with (*Evaler).prepareFrame.
- Remove reliance on scopeOp, concentrating all the scope for creating the local
scope in (*closure).call.
- Check options at the very beginning, and include all unsupported options in the
error.
The -source command now runs with a temporary namespace that is amalgamated
from the local and up namespace of the caller -source; this means that it can
no longer mutate its caller's local scope, which is the only possible sensible
behavior anyway.
This fixes#1202.
Most of the places that need to directly call a function is in the edit package,
which need to call user-defined callbacks.
This change eliminates most call sites of NewTopFrame (including all call sites
outside the eval package). Remove the function and inline it in the remaining
few call sites.
Remove NewTopFrame means that the eval package no longer offers other packages
a way to construct Frame instances. This is intended: Frame is a relatively
low-level concept, and all code outside the eval package now uses the more
high-level Eval, Call, Check/CheckTree methods of *Evaler. The most notable
exception is packages that implement modules; they may still use Frame to access
the information kept in it, but they never construct Frame instances.
In future, the Frame type can be changed to an interface.
This feature supersedes the CompileWithGlobal method, and simplifies the only
use of that method in pkg/edit, where the default binding is evaluated using the
edit: namespace as the global Ns.
The Compile method was used only by the syntax highlighting code to find
compilation errors. Since it only needs the error part but not the Op part,
provide an alternative API that only exposes the error.
The new (*Evaler).Check method allows the implementation to be simpler.
Also test the JSON format feature in an integration test instead of unit test;
this makes it unnecessary for the eval package to expose NewCompilationError.
This method has the property that it always tries to compile the code even if
there is a parse failure. This is the more desirable behavior when checking
code: if there is a parse failure near the end of a chunk of code, the user may
like to learn about compile errors earlier in the code.
The test contained a race condition: when the mock TimeAfter implementation
sends the process a SIGINT, the "sleep" function may not have entered the
"select" block, meaning that the signal will be ignored.
This commit works around this by waiting 1ms (scaled with
$ELVISH_TEST_TIME_SCALE) before sending SIGINT. The test now consistently
succeeds on my local laptop.
Introduces two functions, PipePort and CapturePort, and implement output capture
in terms of them. These two functions return *Port instances, which can also be
used in (*Evaler).Eval calls.
I noticed that most places in the documentation that emit a "note"
about Elvish behavior use `**Note**:`. There were two places that use
all uppercase. Make those consistent with the other uses. Also clarify
the note associated with the `not` builtin.
Including the float64 data type in the prompt value stream causes an error.
This causes those values to be implicitly converted to a string as happens
everywhere else in Elvish.
I initially intended to modify the code to do the implicit string coercion
for any type not otherwise explicitly handled by the pkg/ui `Concat`
methods. I decided against that approach because doing so doesn't make
sense for some types that might appear in the value stream; e.g., Elvish
exceptions.
Fixes#1186
Commit 734eb95 changed most uses of `fm.ports[x].` to a public method
that makes the intent clearer. This changes a couple of uses that were
overlooked by that prior change.
As with commit #eb2a792 I acknowledge that `golint` recommendations are
controversial and should not automatically be acted on. Nonetheless,
this change fixes legitimate lint issues such as copy/paste cleanups,
style problems I introduced (`i += 1` versus `i++`) or method/var comments
that are not in the preferred form.
There are some Navigation mode corner cases whose current behavior is
surprising. Such as inserting a space before the filename when the space
isn't wanted or redundant. Also, selecting an empty directory and pressing
Enter should insert nothing rather than an empty string.
Fixes#1169
Not sure what happened but my fix for issue #1120 resulted in the core
unit test module not being executed due to a bogus "// +build wtf"
comment. Apparently I was experimenting with "+build" directives and
forgot to remove that line. This also improves the test coverage of the
$pwd code from 44% to 100%.
Fixing issue #857 will require some non-trivial work. But I noticed these
documentation cleanups when I started working on the issue so I'm making
them as an independent change.
Related #857
Most of the code uses keyed fields in composite literals; i.e., struct
literals. However, running `go vet ./...` reports a few places that use
anonymous fields. This modifies those composite literals to use keyed
fields. This does make the code a bit more verbose without reducing
the likelihood of a bug. But it does make the code more consistent, use
best practices, and make it easier to notice if a potential problem is
introduced when running `go vet ./...` since that command now produces
no diagnostic output.
I considered adding a `make vet` target that explicitly ran
go vet -composites=false ./...
I decided not to do that since consistently using keyed composite literals
is preferable to having a mix of keyed and unkeyed composite literals.
This also removes the unused `ExampleLoop` function which causes this
`go vet` warning:
pkg/cli/loop_test.go:130:1: ExampleLoop refers to unknown identifier: Loop
Improve coverage of pkg/eval/external_cmd.go from 58.3% to 86.1% as
measured by `make test` on macOS.
This would have been a smaller change but I felt it was important to
actually validate the exception raised when an external command fails
rather than simply using `.ThrowsAny()`. That necessitated augmenting
the pkg/eval/testutils.go module.
Related #1062
I was reviewing test coverage and noticed that the `esleep`
implementation was undocumented and had no tests. This
a) implements `sleep` and deprecates `esleep`,
b) uses the Go time.ParseDuration function rather than assuming a
simple number of seconds,
c) documents the command,
d) adds tests of the function.
Related #1062
This adds basic tests for three functions in pkg/eval/builtin_fn_cmd.go
that implement the `external`, `has-external`, and `search-external`
builtin commands. TBD is how to test those commands on MS Windows.
Related #1062
I stumbled across a comment that began with "XXX". It was clearly meant as
a "TODO" comment. This changes all such occurrences. However, a few "XXX"
comments are ambiguous and a better prefix might be "WARNING". The "TODO"
prefix at least ensures someone, eventually, looks into the situation
and either rewords the comment or fixes the problem. This change means
everyone can assume searching for "// TODO" will find all such comments
rather than requiring they also know to search for "// XXX".
* pkg/eval/str: move builtin ord and chr to str
Move builtin string function ord and chr to the str module and rename to
to str:to-codepoints and str:from-codepoints respectively as suggested
in #851.
* pkg/eval/str: add from-utf8-bytes & to-utf8-bytes
Add from-utf8-bytes and to-utf8-bytes functions to the str module. This
functions differ from their *-codepoints in that they handle utf8 bytes
instead of whole codepoints. Closes#851
* pkg/eval/str: range check for codepoint and bytes
str:from-codepoints
Add check if arguments codepoints are within valid unicode range, return
an OutOfRange error otherwise. Return a BadValue error if the codepoint
isn't valid. Add/change testcases.
str:from-utf8-bytes
Add check if byte arguments are within valid range, return an OutOfRange
error otherwise. Return a BadValue error if the byte sequence isn't a
valid UTF-8 sequence. Add/change testcases.
Add additional test if piping from str:to-codepoints/str:to-utf8-bytes
to str:from-codpoints/str:from-utf8-bytes returns the original input.
Since `break`, `continue`, and `return` are builtin commands, rather
than language keywords, they should be documented on the builtin module
reference page.
In addition to an uncontroversial spelling fix this addresses several,
related, warnings produced by the `golint` tool. In general I agree with
golint that unnecessary "else" blocks should be avoided. So this change
refactors those cases.
Note: I recognize that `golint` is deprecated (see
https://github.com/golang/go/issues/38968) since it is no longer being
maintained and there is controversy about its set of warnings. Nonetheless,
it appears that the warnings it emits for this project are all reasonable
and actionable with one potential exception: the naming of the `map_`
method in pkg/eval/compile_value.go.
This will likely become a basic operation in future that can support more types
than exceptions, so putting it in the builtin namespace is appropriate.
Also remove the exc: module altogether as it contains nothing now. All the
introspection features are now implemented as fields of the exception values and
the reason values.
* Use a "type" field to identify the type, instead of predicates in the exc:
module.
* Make the reason values behave more like structmaps, including a Repr that
mimics a map.
This fixes#208.
* Fields may now be exposed via getter methods.
* Field names are automatically converted to dashed-case.
* Assoc behavior is no longer implemented for structmaps.
* The IsStructMap marker method no longer takes a special marker argument. There
is a little danger of a value accidentally implementing this method.
This allows filtering the wildcard expansion by path type. For example,
whether the path is a directory, regular file, or something else. So
instead of `find . -type f` you can now do `**[type:file]`.
Resolves#1015
Document that an unqouted `#` character has special meaning to the Elvish
parser in the context of passing `#RGB` values to the `styled` command.
Also document the `fg-` color name prefix.
To facilitate using the `^` character for line continuation we need to
remove its use an an exponentiation function. So introduce `math:pow`
and `math:pow10` as alternatatives and mark `^` as deprecated.
Related #989
The behavior is controlled by a global flag that will be flipped for the release
branch. A flag is available to force deprecations to be shown or hidden.
Commit 8c71635ca3 moved the creation to the start
of pipelines; the approach works for simplistic cases like "x = 1 | nop", but
fails in more complex cases, such as "nop (x = 1) | nop".
The correct place to hoist variable creations is the lexical scope, and this
commit implements this approach.
This fixes#623.
The implementations are mostly copied from the builtin joins, replaces and
splits, with small changes to the error behavior in the join function.
The versions in the builtin module are now deprecated.
Add some comments that clarify why the `count` implmentation does not
use the `Inputs` API. Change the mismatched arg count exception to match
that of the `Inputs` API. Add a couple more unit tests.
Resolves#966
In addition to documenting the `math:` module introduced by commit 2d8a045c
two months ago this also:
a) restructures the code to match that of the `re:` and `str:` modules, and
b) adds many more useful constants and functions.
I don't think this completely addresses issue #727 so I'm marking this
as related to, rather than fixing, that issue.
Related #727
