diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b0fe4ca84..7e3344d5f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,6 +1,6 @@ name: make test -on: [pull_request] +on: [push, pull_request] env: CTEST_PARALLEL_LEVEL: "4" diff --git a/Cargo.toml b/Cargo.toml index bdf8565ee..0adcf7bba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,7 @@ edition = "2021" [profile.release] overflow-checks = true +lto = true [profile.release-with-debug] inherits = "release" @@ -23,7 +24,7 @@ default-run = "fish" # see doc_src/license.rst for details # don't forget to update COPYING and debian/copyright too license = "GPL-2.0-only AND LGPL-2.0-or-later AND MIT AND PSF-2.0" -repository = "https://github.com/krobelus/neofish" +repository = "https://github.com/fish-shell/fish-shell" homepage = "https://fishshell.com" readme = "README.rst" diff --git a/README.rst b/README.rst index bddad2a28..a55ba0ab3 100644 --- a/README.rst +++ b/README.rst @@ -1,69 +1,215 @@ -neofish -======= +.. |Cirrus CI| image:: https://api.cirrus-ci.com/github/fish-shell/fish-shell.svg?branch=master + :target: https://cirrus-ci.com/github/fish-shell/fish-shell + :alt: Cirrus CI Build Status -This is a soft fork of `fish shell `_, a Unix shell for interactive use. +`fish `__ - the friendly interactive shell |Build Status| |Cirrus CI| +============================================================================================= -Our goal is to further improve the interactive shell experience, especially for users who are -already familiar with POSIX shells. +fish is a smart and user-friendly command line shell for macOS, Linux, +and the rest of the family. fish includes features like syntax +highlighting, autosuggest-as-you-type, and fancy tab completions that +just work, with no configuration required. -Most of the current changes add well-known syntax present in other shells. Just like ZSH, neofish -will not be strictly POSIX compatible (at least not in the default mode). For scripting, we -recommend using ``/bin/sh`` instead. +For downloads, screenshots and more, go to https://fishshell.com/. -Our ``master`` branch is frequently rebased on `upstream -`_ and force-pushed. This makes it easy to both see the -net difference and to reintegrate any changes into upstream, which we're happy work on. Maybe some -features should be opt-in? +Quick Start +----------- -Here is an overview of the changes, at various stages of completion. -Some already integrated into upstream, some others are functional but lack polish. +fish generally works like other shells, like bash or zsh. A few +important differences can be found at +https://fishshell.com/docs/current/tutorial.html by searching for the +magic phrase “unlike other shells”. -- ☑ ``$()`` command substitutions -- ☑ ``$()`` in command position (to compute the command to run) -- ☑ ``{}`` compound commands -- ☑ ``()`` subshells -- ☐ ``$(())`` arithmetic expansion -- ☑ control flow like ``if foo; then; fi`` and ``for; do; done``. -- ☑ ``foo=bar`` variable overrides -- ☑ ``foo=bar`` global variable assignments -- ☐ ``foo() { :; }`` function definitions -- ☐ POSIX-style single quotes -- ☐ POSIX-like heredocs (`cat << EOF`) -- ☑ variables like ``$?``, ``$$``, ``$#`` and ``$@`` -- ☐ ``${foo#prefix}`` variable expansion -- ☐ process substitution (``<(foo)``) -- ☐ maybe add nestable and raw quoting syntax -- ☑ stop overriding ``MANPATH`` which shadows docs of POSIX utilties like ``exec`` -- ☑ familiar abbreviations for modifier keys like ``bind c-x`` for ``bind ctrl-x`` +Detailed user documentation is available by running ``help`` within +fish, and also at https://fishshell.com/docs/current/index.html -Installation +Getting fish ------------ -Compiling fish requires: +macOS +~~~~~ -- Rust (version 1.70 or later) -- a C compiler -- Sphinx (to build documentation) +fish can be installed: -.. code:: shell +- using `Homebrew `__: ``brew install fish`` +- using `MacPorts `__: + ``sudo port install fish`` +- using the `installer from fishshell.com `__ +- as a `standalone app from fishshell.com `__ - git clone https://github.com/krobelus/neofish - cd neofish - cargo install --path . - ~/.cargo/bin/fish - # configure your terminal to run that binary +Note: The minimum supported macOS version is 10.10 "Yosemite". -Optional Dependencies ---------------------- +Packages for Linux +~~~~~~~~~~~~~~~~~~ + +Packages for Debian, Fedora, openSUSE, and Red Hat Enterprise +Linux/CentOS are available from the `openSUSE Build +Service `__. + +Packages for Ubuntu are available from the `fish +PPA `__, +and can be installed using the following commands: + +:: + + sudo apt-add-repository ppa:fish-shell/release-3 + sudo apt update + sudo apt install fish + +Instructions for other distributions may be found at +`fishshell.com `__. + +Windows +~~~~~~~ + +- On Windows 10/11, fish can be installed under the WSL Windows Subsystem + for Linux with the instructions for the appropriate distribution + listed above under “Packages for Linux”, or from source with the + instructions below. +- fish (4.0 on and onwards) cannot be installed in Cygwin, due to a lack of Rust support. + +Building from source +~~~~~~~~~~~~~~~~~~~~ + +If packages are not available for your platform, GPG-signed tarballs are +available from `fishshell.com `__ and +`fish-shell on +GitHub `__. See the +`Building <#building>`__ section for instructions. + +Running fish +------------ + +Once installed, run ``fish`` from your current shell to try fish out! + +Dependencies +~~~~~~~~~~~~ + +Running fish requires: + +- A terminfo database, typically from curses or ncurses (preinstalled on most \*nix systems) - this needs to be the directory tree format, not the "hashed" database. + If this is unavailable, fish uses an included xterm-256color definition. +- some common \*nix system utilities (currently ``mktemp``), in + addition to the basic POSIX utilities (``cat``, ``cut``, ``dirname``, + ``file``, ``ls``, ``mkdir``, ``mkfifo``, ``rm``, ``sort``, ``tee``, ``tr``, + ``uname`` and ``sed`` at least, but the full coreutils plus ``find`` and + ``awk`` is preferred) +- The gettext library, if compiled with + translation support The following optional features also have specific requirements: - builtin commands that have the ``--help`` option or print usage - messages require ``nroff`` or ``mandoc`` for display + messages require ``nroff`` or ``mandoc`` for + display - automated completion generation from manual pages requires Python 3.5+ - the ``fish_config`` web configuration tool requires Python 3.5+ and a web browser - system clipboard integration (with the default Ctrl-V and Ctrl-X bindings) require either the ``xsel``, ``xclip``, ``wl-copy``/``wl-paste`` or ``pbcopy``/``pbpaste`` utilities +- full completions for ``yarn`` and ``npm`` require the + ``all-the-package-names`` NPM module +- ``colorls`` is used, if installed, to add color when running ``ls`` on platforms + that do not have color support (such as OpenBSD) + +Building +-------- + +.. _dependencies-1: + +Dependencies +~~~~~~~~~~~~ + +Compiling fish requires: + +- Rust (version 1.70 or later) +- CMake (version 3.15 or later) +- a C compiler (for system feature detection and the test helper binary) +- PCRE2 (headers and libraries) - optional, this will be downloaded if missing +- gettext (headers and libraries) - optional, for translation support +- an Internet connection, as other dependencies will be downloaded automatically + +Sphinx is also optionally required to build the documentation from a +cloned git repository. Additionally, running the full test suite requires Python 3, tmux, and the pexpect package. + +Building from source with CMake +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Rather than building from source, consider using a packaged build for your platform. Using the +steps below makes fish difficult to uninstall or upgrade. Release packages are available from the +links above, and up-to-date `development builds of fish are available for many platforms +`__ + +To install into ``/usr/local``, run: + +.. code:: bash + + mkdir build; cd build + cmake .. + cmake --build . + sudo cmake --install . + +The install directory can be changed using the +``-DCMAKE_INSTALL_PREFIX`` parameter for ``cmake``. + +CMake Build options +~~~~~~~~~~~~~~~~~~~ + +In addition to the normal CMake build options (like ``CMAKE_INSTALL_PREFIX``), fish's CMake build has some other options available to customize it. + +- BUILD_DOCS=ON|OFF - whether to build the documentation. This is automatically set to OFF when Sphinx isn't installed. +- INSTALL_DOCS=ON|OFF - whether to install the docs. This is automatically set to on when BUILD_DOCS is or prebuilt documentation is available (like when building in-tree from a tarball). +- FISH_USE_SYSTEM_PCRE2=ON|OFF - whether to use an installed pcre2. This is normally autodetected. +- MAC_CODESIGN_ID=String|OFF - the codesign ID to use on Mac, or "OFF" to disable codesigning. +- WITH_GETTEXT=ON|OFF - whether to build with gettext support for translations. +- extra_functionsdir, extra_completionsdir and extra_confdir - to compile in an additional directory to be searched for functions, completions and configuration snippets + +Building fish as self-installable (experimental) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +You can also build fish as a self-installing binary. + +This will include all the datafiles like the included functions or web configuration tool in the main ``fish`` binary. + +On the first interactive run, and whenever it notices they are out of date, it will extract the datafiles to ~/.local/share/fish/install/ (currently, subject to change). You can do this manually by running ``fish --install``. + +To install fish as self-installable, just use ``cargo``, like:: + + cargo install --path /path/to/fish # if you have a git clone + cargo install --git https://github.com/fish-shell/fish-shell --tag 4.0 # to build from git once 4.0 is released + cargo install --git https://github.com/fish-shell/fish-shell # to build the current development snapshot without cloning + +This will place the binaries in ``~/.cargo/bin/``, but you can place them wherever you want. + +This build won't have the HTML docs (``help`` will open the online version) or translations. + +It will try to build the man pages with sphinx-build. If that is not available and you would like to include man pages, you need to install it and retrigger the build script, e.g. by setting FISH_BUILD_DOCS=1:: + + FISH_BUILD_DOCS=1 cargo install --path . + +Setting it to "0" disables the inclusion of man pages. + +You can also link this build statically (but not against glibc) and move it to other computers. + +Contributing Changes to the Code +-------------------------------- + +See the `Guide for Developers `__. + +Contact Us +---------- + +Questions, comments, rants and raves can be posted to the official fish +mailing list at https://lists.sourceforge.net/lists/listinfo/fish-users +or join us on our `matrix +channel `__. Or use the `fish tag +on Unix & Linux Stackexchange `__. +There is also a fish tag on Stackoverflow, but it is typically a poor fit. + +Found a bug? Have an awesome idea? Please `open an +issue `__. + +.. |Build Status| image:: https://github.com/fish-shell/fish-shell/workflows/make%20test/badge.svg + :target: https://github.com/fish-shell/fish-shell/actions diff --git a/build_tools/osx_package_resources/welcome.html b/build_tools/osx_package_resources/welcome.html index 3d84dcb56..549a1d280 100644 --- a/build_tools/osx_package_resources/welcome.html +++ b/build_tools/osx_package_resources/welcome.html @@ -23,6 +23,6 @@

chsh -s /usr/local/bin/fish

-

Enjoy! Bugs can be reported on GitHub.

+

Enjoy! Bugs can be reported on GitHub.

diff --git a/debian/control b/debian/control index 325dbdfc2..a78b18011 100644 --- a/debian/control +++ b/debian/control @@ -15,8 +15,8 @@ Build-Depends: debhelper (>= 12), python3 Standards-Version: 4.1.5 Homepage: https://fishshell.com/ -Vcs-Git: https://github.com/krobelus/neofish.git -Vcs-Browser: https://github.com/krobelus/neofish +Vcs-Git: https://github.com/fish-shell/fish-shell.git +Vcs-Browser: https://github.com/fish-shell/fish-shell Package: fish Architecture: any diff --git a/doc_src/cmds/bind.rst b/doc_src/cmds/bind.rst index 554deb1f9..914a30b61 100644 --- a/doc_src/cmds/bind.rst +++ b/doc_src/cmds/bind.rst @@ -23,7 +23,7 @@ If both ``KEYS`` and ``COMMAND`` are given, ``bind`` adds (or replaces) a bindin If only ``KEYS`` is given, any existing binding in the given ``MODE`` will be printed. ``KEYS`` is a comma-separated list of key names. -Modifier keys can be specified by prefixing a key name with a combination of ``ctrl-``/``c-``, ``alt-``/``a-`` and ``shift-``. +Modifier keys can be specified by prefixing a key name with a combination of ``ctrl-``, ``alt-`` and ``shift-``. For example, pressing :kbd:`w` while holding the Alt modifier is written as ``alt-w``. Key names are case-sensitive; for example ``alt-W`` is the same as ``alt-shift-w``. ``ctrl-x,ctrl-e`` would mean pressing :kbd:`ctrl-x` followed by :kbd:`ctrl-e`. diff --git a/printf/Cargo.toml b/printf/Cargo.toml index b221edff1..cf387ec72 100644 --- a/printf/Cargo.toml +++ b/printf/Cargo.toml @@ -2,7 +2,7 @@ name = "fish-printf" edition = "2021" version = "0.2.1" -repository = "https://github.com/krobelus/neofish" +repository = "https://github.com/fish-shell/fish-shell" description = "printf implementation, based on musl" license = "MIT" diff --git a/share/functions/man.fish b/share/functions/man.fish index 8b1378917..ac1ec367f 100644 --- a/share/functions/man.fish +++ b/share/functions/man.fish @@ -1 +1,55 @@ +if not command -qs man + # see #5329 and discussion at https://github.com/fish-shell/fish-shell/commit/13e025bdb01cc4dd08463ec497a0a3495873702f + exit +end +function man --description "Format and display the on-line manual pages" + # Work around the "builtin" manpage that everything symlinks to, + # by prepending our fish datadir to man. This also ensures that man gives fish's + # man pages priority, without having to put fish's bin directories first in $PATH. + + # Preserve the existing MANPATH, and default to the system path (the empty string). + set -l manpath + if set -q MANPATH + set manpath $MANPATH + else if set -l p (command man -p 2>/dev/null) + # NetBSD's man uses "-p" to print the path. + # FreeBSD's man also has a "-p" option, but that requires an argument. + # Other mans (men?) don't seem to have it. + # + # Unfortunately NetBSD prints things like "/usr/share/man/man1", + # while not allowing them as $MANPATH components. + # What it needs is just "/usr/share/man". + # + # So we strip the last component. + # This leaves a few wrong directories, but that should be harmless. + set manpath (string replace -r '[^/]+$' '' $p) + else + set manpath '' + end + # Notice the shadowing local exported copy of the variable. + set -lx MANPATH $manpath + + # Prepend fish's man directory if available. + set -l fish_manpath $__fish_data_dir/man + if test -d $fish_manpath + set MANPATH $fish_manpath $MANPATH + end + + if test (count $argv) -eq 1 + # Some of these don't have their own page, + # and adding one would be awkward given that the filename + # isn't guaranteed to be allowed. + # So we override them with the good name. + switch $argv + case : + set argv true + case '[' + set argv test + case . + set argv source + end + end + + command man $argv +end diff --git a/src/ast.rs b/src/ast.rs index 7a21a7875..0136f3762 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -14,7 +14,7 @@ use crate::parse_constants::{ token_type_user_presentable_description, ParseError, ParseErrorCode, ParseErrorList, ParseKeyword, ParseTokenType, ParseTreeFlags, SourceRange, StatementDecoration, - INVALID_PIPELINE_CMD_ERR_MSG, SOURCE_OFFSET_INVALID, + ERROR_BAD_COMMAND_ASSIGN_ERR_MSG, INVALID_PIPELINE_CMD_ERR_MSG, SOURCE_OFFSET_INVALID, }; use crate::parse_tree::ParseToken; #[cfg(test)] @@ -55,12 +55,7 @@ pub struct MissingEndError { token: ParseToken, } -pub enum NonLocalError { - MissingEndError(MissingEndError), - MissingStatementError, -} - -pub type VisitResult = ControlFlow; +pub type VisitResult = ControlFlow; trait NodeVisitorMut { /// will_visit (did_visit) is called before (after) a node's fields are visited. @@ -90,7 +85,6 @@ fn visit_decorated_statement_decorator( fn visit_then(&mut self, _node: &mut Option); fn visit_time(&mut self, _node: &mut Option); fn visit_token_background(&mut self, _node: &mut Option); - fn visit_optional_statement(&mut self, _node: &mut Option); } trait AcceptorMut { @@ -191,9 +185,6 @@ fn as_argument_or_redirection_list(&self) -> Option<&ArgumentOrRedirectionList> fn as_command_token(&self) -> Option<&CommandToken> { None } - fn as_variable_statement(&self) -> Option<&VariableStatement> { - None - } fn as_statement(&self) -> Option<&Statement> { None } @@ -318,9 +309,6 @@ fn as_mut_argument_or_redirection_list(&mut self) -> Option<&mut ArgumentOrRedir fn as_mut_command_token(&mut self) -> Option<&mut CommandToken> { None } - fn as_mut_variable_statement(&mut self) -> Option<&mut VariableStatement> { - None - } fn as_mut_statement(&mut self) -> Option<&mut Statement> { None } @@ -1000,9 +988,6 @@ macro_rules! visit_optional_field_mut { (TokenBackground, $field:expr, $visitor:ident) => { $visitor.visit_token_background(&mut $field); }; - (Statement, $field:expr, $visitor:ident) => { - $visitor.visit_optional_statement(&mut $field); - }; } macro_rules! visit_result { @@ -1242,31 +1227,6 @@ pub fn is_left_brace(&self) -> bool { } } -#[derive(Default, Debug)] -pub struct VariableStatement { - parent: Option<*const dyn Node>, - /// A (possibly empty) list of variable assignments. - pub variables: VariableAssignmentList, - /// The statement. - pub statement: Option, -} -implement_node!(VariableStatement, branch, variable_statement); -implement_acceptor_for_branch!( - VariableStatement, - (variables: (VariableAssignmentList)), - (statement: (Option)), -); -impl ConcreteNode for VariableStatement { - fn as_variable_statement(&self) -> Option<&VariableStatement> { - Some(self) - } -} -impl ConcreteNodeMut for VariableStatement { - fn as_mut_variable_statement(&mut self) -> Option<&mut VariableStatement> { - Some(self) - } -} - /// A statement is a normal command, or an if / while / etc #[derive(Default, Debug)] pub struct Statement { @@ -1286,16 +1246,6 @@ fn as_mut_statement(&mut self) -> Option<&mut Statement> { } } -impl CheckParse for Statement { - fn can_be_parsed(pop: &mut Populator<'_>) -> bool { - let next = pop.peek_token(0); - matches!( - next.typ, - ParseTokenType::terminate | ParseTokenType::left_brace - ) || (next.typ == ParseTokenType::string && !next.is_variable_assignment) - } -} - /// A job is a non-empty list of statements, separated by pipes. (Non-empty is useful for cases /// like if statements, where we require a command). #[derive(Default, Debug)] @@ -1303,7 +1253,10 @@ pub struct JobPipeline { parent: Option<*const dyn Node>, /// Maybe the time keyword. pub time: Option, - pub statement2: VariableStatement, + /// A (possibly empty) list of variable assignments. + pub variables: VariableAssignmentList, + /// The statement. + pub statement: Statement, /// Piped remainder. pub continuation: JobContinuationList, /// Maybe backgrounded. @@ -1313,7 +1266,8 @@ pub struct JobPipeline { implement_acceptor_for_branch!( JobPipeline, (time: (Option)), - (statement2: (VariableStatement)), + (variables: (VariableAssignmentList)), + (statement: (Statement)), (continuation: (JobContinuationList)), (bg: (Option)), ); @@ -1327,11 +1281,6 @@ fn as_mut_job_pipeline(&mut self) -> Option<&mut JobPipeline> { Some(self) } } -impl JobPipeline { - pub fn statement3(&self) -> Option<&Statement> { - self.statement2.statement.as_ref() - } -} /// A job_conjunction is a job followed by a && or || continuations. #[derive(Default, Debug)] @@ -1810,14 +1759,16 @@ pub struct NotStatement { /// Keyword, either not or exclam. pub kw: KeywordNot, pub time: Option, - pub negated_statement: VariableStatement, + pub variables: VariableAssignmentList, + pub contents: Statement, } implement_node!(NotStatement, branch, not_statement); implement_acceptor_for_branch!( NotStatement, (kw: (KeywordNot)), (time: (Option)), - (negated_statement: (VariableStatement)), + (variables: (VariableAssignmentList)), + (contents: (Statement)), ); impl ConcreteNode for NotStatement { fn as_not_statement(&self) -> Option<&NotStatement> { @@ -1835,14 +1786,16 @@ pub struct JobContinuation { parent: Option<*const dyn Node>, pub pipe: TokenPipe, pub newlines: MaybeNewlines, - pub statement2: VariableStatement, + pub variables: VariableAssignmentList, + pub statement: Statement, } implement_node!(JobContinuation, branch, job_continuation); implement_acceptor_for_branch!( JobContinuation, (pipe: (TokenPipe)), (newlines: (MaybeNewlines)), - (statement2: (VariableStatement)), + (variables: (VariableAssignmentList)), + (statement: (Statement)), ); impl ConcreteNode for JobContinuation { fn as_job_continuation(&self) -> Option<&JobContinuation> { @@ -1859,11 +1812,6 @@ fn can_be_parsed(pop: &mut Populator<'_>) -> bool { pop.peek_type(0) == ParseTokenType::pipe } } -impl JobContinuation { - pub fn statement(&self) -> Option<&Statement> { - self.statement2.statement.as_ref() - } -} define_list_node!(JobContinuationList, job_continuation_list, JobContinuation); impl ConcreteNode for JobContinuationList { @@ -2062,18 +2010,20 @@ fn as_mut_variable_assignment(&mut self) -> Option<&mut VariableAssignment> { impl CheckParse for VariableAssignment { fn can_be_parsed(pop: &mut Populator<'_>) -> bool { // Do we have a variable assignment at all? - pop.peek_token(0).is_variable_assignment - // // What is the token after it? - // match pop.peek_type(1) { - // // We have `a= cmd` and should treat it as a variable assignment. - // ParseTokenType::string | ParseTokenType::left_brace => true, - // // We have `a=` which is OK if we are allowing incomplete, an error otherwise. - // ParseTokenType::terminate => pop.allow_incomplete(), - // // We have e.g. `a= >` which is an error. - // // Note that we do not produce an error here. Instead we return false - // // so this the token will be seen by allocate_populate_statement_contents. - // _ => false, - // } + if !pop.peek_token(0).may_be_variable_assignment { + return false; + } + // What is the token after it? + match pop.peek_type(1) { + // We have `a= cmd` and should treat it as a variable assignment. + ParseTokenType::string | ParseTokenType::left_brace => true, + // We have `a=` which is OK if we are allowing incomplete, an error otherwise. + ParseTokenType::terminate => pop.allow_incomplete(), + // We have e.g. `a= >` which is an error. + // Note that we do not produce an error here. Instead we return false + // so this the token will be seen by allocate_populate_statement_contents. + _ => false, + } } } @@ -2628,7 +2578,6 @@ pub fn ast_type_to_string(t: Type) -> &'static wstr { Type::argument_or_redirection => L!("argument_or_redirection"), Type::argument_or_redirection_list => L!("argument_or_redirection_list"), Type::command_token => L!("command_token"), - Type::variable_statement => L!("variable_statement"), Type::statement => L!("statement"), Type::job_pipeline => L!("job_pipeline"), Type::job_conjunction => L!("job_conjunction"), @@ -2970,7 +2919,7 @@ fn advance_1(&mut self) -> ParseToken { result.has_dash_prefix = text.starts_with('-'); result.is_help_argument = [L!("-h"), L!("--help")].contains(&text); result.is_newline = result.typ == ParseTokenType::end && text == "\n"; - result.is_variable_assignment = variable_assignment_equals_pos(text).is_some(); + result.may_be_variable_assignment = variable_assignment_equals_pos(text).is_some(); result.tok_error = token.error; assert!(token.offset() < SOURCE_OFFSET_INVALID); @@ -3080,9 +3029,6 @@ struct Populator<'a> { /// Stream of tokens which we consume. tokens: TokenStream<'a>, - /// Whether this statement has a prefixed variable override. - parsing_variable_statement: Option, - /** The type which we are attempting to parse, typically job_list but may be freestanding_argument_list. */ top_type: Type, @@ -3134,42 +3080,9 @@ fn visit_mut(&mut self, node: &mut dyn NodeMut) -> VisitResult { Category::leaf => {} // Visit branch nodes by just calling accept() to visit their fields. Category::branch => { - self.parsing_variable_statement = Some( - node.as_variable_statement().is_some() - && self.peek_token(0).is_variable_assignment, - ); // This field is a direct embedding of an AST value. node.accept_mut(self, false); - let Some(variable_statement) = node.as_mut_variable_statement() else { - return VisitResult::Continue(()); - }; - if self.peek_token(0).typ == ParseTokenType::terminate { - return VisitResult::Continue(()); - } - if variable_statement.statement.as_ref().is_some_and(|stmt| - // TODO(posix_mode) # Un-clone - // This may happen if we just have a 'time' prefix. - // Construct a decorated statement, which will be unsourced. - stmt - .contents - .as_decorated_statement() - .is_some_and(|ds| ds.try_source_range().is_none())) - { - variable_statement.statement = None; - } - if variable_statement.statement.is_some() - || !variable_statement.variables.is_empty() - { - return VisitResult::Continue(()); - } - parse_error!( - self, - self.peek_token(0), - ParseErrorCode::generic, - "asdf Expected a command but found %ls", - self.peek_token(0).user_presentable_description() - ); - return VisitResult::Break(NonLocalError::MissingStatementError); + return VisitResult::Continue(()); } Category::list => { // This field is an embedding of an array of (pointers to) ContentsNode. @@ -3237,9 +3150,6 @@ fn did_visit_fields_of<'a>(&'a mut self, node: &'a dyn NodeMut, flow: VisitResul let VisitResult::Break(error) = flow else { return; }; - let NonLocalError::MissingEndError(error) = error else { - return; - }; let token = &error.token; // To-do: maybe extend this to other tokenizer errors? @@ -3394,14 +3304,6 @@ fn visit_then(&mut self, node: &mut Option) { fn visit_token_background(&mut self, node: &mut Option) { *node = self.try_parse::().map(|b| *b); } - fn visit_optional_statement(&mut self, node: &mut Option) { - if self.parsing_variable_statement.take().unwrap() - && self.peek_token(0).typ == ParseTokenType::terminate - { - return; - } - *node = self.try_parse::().map(|b| *b); - } } /// Helper to describe a list of keywords. @@ -3448,7 +3350,6 @@ fn new( semis: vec![], errors: vec![], tokens: TokenStream::new(src, flags, top_type == Type::freestanding_argument_list), - parsing_variable_statement: None, top_type, unwinding: false, any_error: false, @@ -3957,26 +3858,25 @@ fn new_decorated_statement(slf: &mut Populator<'_>) -> StatementVariant { self.peek_token(0).user_presentable_description() ); return got_error(self); - } else if self.peek_token(0).is_variable_assignment { + } else if self.peek_token(0).may_be_variable_assignment { // Here we have a variable assignment which we chose to not parse as a variable // assignment because there was no string after it. // Ensure we consume the token, so we don't get back here again at the same place. - // let token = &self.consume_any_token(); - // let text = &self.tokens.src - // [token.source_start()..token.source_start() + token.source_length()]; - // let equals_pos = variable_assignment_equals_pos(text).unwrap(); - // let variable = &text[..equals_pos]; - // let value = &text[equals_pos + 1..]; - // parse_error!( - // self, - // token, - // ParseErrorCode::bare_variable_assignment, - // ERROR_BAD_COMMAND_ASSIGN_ERR_MSG, - // variable, - // value - // ); - // return got_error(self); - return new_decorated_statement(self); + let token = &self.consume_any_token(); + let text = &self.tokens.src + [token.source_start()..token.source_start() + token.source_length()]; + let equals_pos = variable_assignment_equals_pos(text).unwrap(); + let variable = &text[..equals_pos]; + let value = &text[equals_pos + 1..]; + parse_error!( + self, + token, + ParseErrorCode::bare_variable_assignment, + ERROR_BAD_COMMAND_ASSIGN_ERR_MSG, + variable, + value + ); + return got_error(self); } // In some cases a block starter is a decorated statement instead, mostly if invoked with "--help". @@ -4135,7 +4035,7 @@ fn visit_variable_assignment(&mut self, varas: &mut VariableAssignment) { varas.range = None; return; } - if !self.peek_token(0).is_variable_assignment { + if !self.peek_token(0).may_be_variable_assignment { internal_error!( self, visit_variable_assignment, @@ -4216,10 +4116,10 @@ fn visit_keyword(&mut self, keyword: &mut dyn Keyword) -> VisitResult { // Special error reporting for keyword_t. let allowed_keywords = keyword.allowed_keywords(); if allowed_keywords.contains(&ParseKeyword::kw_end) { - return VisitResult::Break(NonLocalError::MissingEndError(MissingEndError { + return VisitResult::Break(MissingEndError { allowed_keywords, token: *self.peek_token(0), - })); + }); } else { parse_error!( self, @@ -4434,7 +4334,6 @@ pub enum Type { argument_or_redirection, argument_or_redirection_list, command_token, - variable_statement, statement, job_pipeline, job_conjunction, diff --git a/src/builtins/fish_indent.rs b/src/builtins/fish_indent.rs index b7ebb2953..90125b3f4 100644 --- a/src/builtins/fish_indent.rs +++ b/src/builtins/fish_indent.rs @@ -349,20 +349,18 @@ fn gap_text_flags_before_node(&self, node: &dyn Node) -> GapFlags { let p = p.parent().unwrap(); assert_eq!(p.typ(), Type::statement); let p = p.parent().unwrap(); - #[allow(clippy::manual_map)] - if (if let Some(job) = p.as_job_pipeline() { - Some(&job.statement2) + if let Some(job) = p.as_job_pipeline() { + if !job.variables.is_empty() { + result.allow_escaped_newlines = true; + } } else if let Some(job_cnt) = p.as_job_continuation() { - Some(&job_cnt.statement2) + if !job_cnt.variables.is_empty() { + result.allow_escaped_newlines = true; + } } else if let Some(not_stmt) = p.as_not_statement() { - Some(¬_stmt.negated_statement) - } else { - None - }) - .is_some_and(|stmt| !stmt.variables.is_empty()) - { - result.allow_escaped_newlines = true; - result.allow_escaped_newlines = true; + if !not_stmt.variables.is_empty() { + result.allow_escaped_newlines = true; + } } } _ => (), diff --git a/src/common.rs b/src/common.rs index cb1764cc6..d4a8a32da 100644 --- a/src/common.rs +++ b/src/common.rs @@ -584,17 +584,9 @@ enum Mode { } '$' => { if unescape_special { - let next = input.char_at(input_position + 1); - let is_cmdsub = input_position + 1 < input.len() && next == '('; - let is_pid = !valid_var_name_char(next) - && result - .as_char_slice() - .last() - .map(|prev| { - [VARIABLE_EXPAND, VARIABLE_EXPAND_SINGLE].contains(prev) - }) - .unwrap_or_default(); - if !is_cmdsub && !is_pid { + let is_cmdsub = input_position + 1 < input.len() + && input.char_at(input_position + 1) == '('; + if !is_cmdsub { to_append_or_none = Some(VARIABLE_EXPAND); vars_or_seps.push(input_position); } diff --git a/src/expand.rs b/src/expand.rs index 398de2e34..9e912dfc1 100644 --- a/src/expand.rs +++ b/src/expand.rs @@ -3,8 +3,6 @@ //! from using a more clever memory allocation scheme, perhaps an evil combination of talloc, //! string buffers and reference counting. -use std::sync::atomic::Ordering; - use crate::builtins::shared::{ STATUS_CMD_ERROR, STATUS_CMD_UNKNOWN, STATUS_EXPAND_ERROR, STATUS_ILLEGAL_CMD, STATUS_INVALID_ARGS, STATUS_NOT_EXECUTABLE, STATUS_READ_TOO_MUCH, STATUS_UNMATCHED_WILDCARD, @@ -15,18 +13,16 @@ UnescapeFlags, UnescapeStringStyle, EXPAND_RESERVED_BASE, EXPAND_RESERVED_END, }; use crate::complete::{CompleteFlags, Completion, CompletionList, CompletionReceiver}; -use crate::env::{EnvVar, EnvVarFlags, Environment}; +use crate::env::{EnvVar, Environment}; use crate::exec::exec_subshell_for_expand; use crate::future_feature_flags::{feature_test, FeatureFlag}; use crate::history::{history_session_id, History}; -use crate::libc::_PATH_BSHELL; use crate::operation_context::OperationContext; use crate::parse_constants::{ParseError, ParseErrorCode, ParseErrorList, SOURCE_LOCATION_UNKNOWN}; use crate::parse_util::{ parse_util_expand_variable_error, parse_util_locate_cmdsubst_range, MaybeParentheses, }; use crate::path::path_apply_working_directory; -use crate::threads::MainThread; use crate::util::wcsfilecmp_glob; use crate::wchar::prelude::*; use crate::wcstringutil::{join_strings, trim}; @@ -34,7 +30,6 @@ use crate::wildcard::{WildcardResult, ANY_CHAR, ANY_STRING, ANY_STRING_RECURSIVE}; use crate::wutil::{normalize_path, wcstoi_partial, Options}; use bitflags::bitflags; -use once_cell::unsync::OnceCell; bitflags! { /// Set of flags controlling expansions. @@ -79,8 +74,6 @@ pub struct ExpandFlags : u16 { const SPECIAL_FOR_COMMAND = 1 << 13; /// The token has an unclosed brace, so don't add a space. const NO_SPACE_FOR_UNCLOSED_BRACE = 1 << 14; - /// TODO - const FOR_COMMAND = 1 << 15; } } @@ -232,11 +225,7 @@ pub fn expand_to_command_and_args( return ExpandResult::ok(); } - let mut eflags = if ctx.has_parser() { - ExpandFlags::FOR_COMMAND - } else { - ExpandFlags::FAIL_ON_CMDSUBST - }; + let mut eflags = ExpandFlags::FAIL_ON_CMDSUBST; if skip_wildcards { eflags |= ExpandFlags::SKIP_WILDCARDS; } @@ -377,6 +366,20 @@ macro_rules! append_syntax_error { } } +/// Append a cmdsub error to the given error list. But only do so if the error hasn't already been +/// recorded. This is needed because command substitution is a recursive process and some errors +/// could consequently be recorded more than once. +macro_rules! append_cmdsub_error { + ( + $errors:expr, $source_start:expr, $source_end:expr, + $fmt:expr $(, $arg:expr )* $(,)? + ) => { + append_cmdsub_error_formatted!( + $errors, $source_start, $source_end, + wgettext_maybe_fmt!($fmt $(, $arg)*)); + } +} + macro_rules! append_cmdsub_error_formatted { ( $errors:expr, $source_start:expr, $source_end:expr, @@ -615,33 +618,7 @@ fn expand_variables( ); // Get the variable name as a string, then try to get the variable from env. - let mut var_name = None; - let mut may_slice = true; - if var_name_stop == var_name_start { - match instr.char_at(var_name_stop) { - '?' => { - var_name_stop += 1; - may_slice = false; - var_name = Some(L!("status")); - } - '$' | VARIABLE_EXPAND | VARIABLE_EXPAND_SINGLE => { - var_name_stop += 1; - may_slice = false; - var_name = Some(L!("fish_pid")); - } - '#' => { - var_name_stop += 1; - may_slice = false; - } - '@' => { - var_name_stop += 1; - may_slice = false; - var_name = Some(L!("argv")); - } - _ => (), - } - } - let var_name = var_name.unwrap_or(&instr[var_name_start..var_name_stop]); + let var_name = &instr[var_name_start..var_name_stop]; // It's an error if the name is empty. if var_name.is_empty() { @@ -663,16 +640,6 @@ fn expand_variables( let mut var = None; if var_name == "history" { history = Some(History::with_name(&history_session_id(vars))); - } else if var_name == "#" { - var = Some(EnvVar::new( - sprintf!( - "%lu", - vars.get(L!("argv")) - .map(|v| v.as_list().len()) - .unwrap_or_default() - ), - EnvVarFlags::default(), - )); } else if var_name.as_char_slice() != [VARIABLE_EXPAND_EMPTY] { var = vars.get(var_name); } @@ -684,7 +651,7 @@ fn expand_variables( let slice_start = var_name_stop; let mut var_idx_list = vec![]; - if may_slice && instr.as_char_slice().get(slice_start) == Some(&'[') { + if instr.as_char_slice().get(slice_start) == Some(&'[') { all_values = false; // If a variable is missing, behave as though we have one value, so that $var[1] always // works. @@ -958,7 +925,6 @@ pub fn expand_cmdsubst( ctx: &OperationContext, out: &mut CompletionReceiver, errors: &mut Option<&mut ParseErrorList>, - for_command: bool, ) -> ExpandResult { let mut cursor = 0; let mut is_quoted = false; @@ -980,23 +946,7 @@ pub fn expand_cmdsubst( } return ExpandResult::ok(); } - MaybeParentheses::CommandSubstitution(parens) => { - if for_command { - // TODO error on extra args - static PATH_BSHELL: MainThread> = - MainThread::new(OnceCell::new()); - let shell = PATH_BSHELL - .get() - .get_or_init(|| charptr2wcstring(_PATH_BSHELL.load(Ordering::Relaxed))); - for arg in [shell.as_utfstr(), L!("-c"), &input[parens.command()]] { - if !out.add(arg.to_owned()) { - return append_overflow_error(errors, None); - } - } - return ExpandResult::ok(); - } - parens - } + MaybeParentheses::CommandSubstitution(parens) => parens, }; let mut sub_res = vec![]; @@ -1102,7 +1052,7 @@ pub fn expand_cmdsubst( tail.insert(0, '"'); } - let _ = expand_cmdsubst(tail, ctx, &mut tail_expand_recv, errors, false); // TODO: offset error locations + let _ = expand_cmdsubst(tail, ctx, &mut tail_expand_recv, errors); // TODO: offset error locations let tail_expand = tail_expand_recv.take(); // Combine the result of the current command substitution with the result of the recursive tail @@ -1376,16 +1326,8 @@ fn stage_cmdsubst(&mut self, input: WString, out: &mut CompletionReceiver) -> Ex return ExpandResult::ok(); } if self.flags.contains(ExpandFlags::FAIL_ON_CMDSUBST) { - // TODO this if FOR_COMMAND let mut cursor = 0; - let mut has_dollar = false; - match parse_util_locate_cmdsubst_range( - &input, - &mut cursor, - true, - None, - Some(&mut has_dollar), - ) { + match parse_util_locate_cmdsubst_range(&input, &mut cursor, true, None, None) { MaybeParentheses::Error => { return ExpandResult::make_error(STATUS_EXPAND_ERROR.unwrap()); } @@ -1395,11 +1337,14 @@ fn stage_cmdsubst(&mut self, input: WString, out: &mut CompletionReceiver) -> Ex } return ExpandResult::ok(); } - MaybeParentheses::CommandSubstitution(_parens) => { - if has_dollar { - return ExpandResult::ok(); - } - return ExpandResult::ok(); + MaybeParentheses::CommandSubstitution(parens) => { + append_cmdsub_error!( + self.errors, + parens.start(), + parens.end()-1, + "command substitutions not allowed in command position. Try var=(your-cmd) $var ..." + ); + return ExpandResult::make_error(STATUS_EXPAND_ERROR.unwrap()); } } } else { @@ -1407,13 +1352,7 @@ fn stage_cmdsubst(&mut self, input: WString, out: &mut CompletionReceiver) -> Ex self.ctx.has_parser(), "Must have a parser to expand command substitutions" ); - expand_cmdsubst( - input, - self.ctx, - out, - self.errors, - self.flags.contains(ExpandFlags::FOR_COMMAND), - ) + expand_cmdsubst(input, self.ctx, out, self.errors) } } diff --git a/src/highlight/highlight.rs b/src/highlight/highlight.rs index 8e39cdf39..16a738b00 100644 --- a/src/highlight/highlight.rs +++ b/src/highlight/highlight.rs @@ -283,7 +283,7 @@ fn autosuggest_parse_command( // Find the first statement. let jc = ast.top().as_job_list().unwrap().get(0)?; - let first_statement = jc.job.statement3()?.contents.as_decorated_statement()?; + let first_statement = jc.job.statement.contents.as_decorated_statement()?; if let Some(expanded_command) = statement_get_expanded_command(buff, first_statement, ctx) { let mut arg = WString::new(); diff --git a/src/key.rs b/src/key.rs index 665dee7eb..22ef27243 100644 --- a/src/key.rs +++ b/src/key.rs @@ -281,8 +281,8 @@ pub(crate) fn parse_keys(value: &wstr) -> Result, WString> { for _i in 0..num_keys.checked_sub(1).unwrap() { let modifier = components.next().unwrap(); match modifier { - _ if modifier == "ctrl" || modifier == "c" => modifiers.ctrl = true, - _ if modifier == "alt" || modifier == "a" => modifiers.alt = true, + _ if modifier == "ctrl" => modifiers.ctrl = true, + _ if modifier == "alt" => modifiers.alt = true, _ if modifier == "shift" => modifiers.shift = true, _ => { return Err(wgettext_fmt!( diff --git a/src/parse_constants.rs b/src/parse_constants.rs index f2e4792fc..7e5888d57 100644 --- a/src/parse_constants.rs +++ b/src/parse_constants.rs @@ -138,11 +138,12 @@ pub enum ParseErrorCode { tokenizer_unterminated_escape, tokenizer_other, - unbalancing_end, // end outside of block - unbalancing_else, // else outside of if - unbalancing_case, // case outside of switch - unbalancing_brace, // } outside of { - andor_in_pipeline, // "and" or "or" after a pipe + unbalancing_end, // end outside of block + unbalancing_else, // else outside of if + unbalancing_case, // case outside of switch + unbalancing_brace, // } outside of { + bare_variable_assignment, // a=b without command + andor_in_pipeline, // "and" or "or" after a pipe } // The location of a pipeline. diff --git a/src/parse_execution.rs b/src/parse_execution.rs index 36971fc58..3b58148ee 100644 --- a/src/parse_execution.rs +++ b/src/parse_execution.rs @@ -450,14 +450,11 @@ fn infinite_recursive_statement_in_job_list<'b>( }; // Check main statement. - let infinite_recursive_statement = statement_recurses(jc.job.statement3()?) + let infinite_recursive_statement = statement_recurses(&jc.job.statement) // Check piped remainder. .or_else(|| { for c in &job.continuation { - let Some(stmt) = c.statement() else { - continue; - }; - let s = statement_recurses(stmt); + let s = statement_recurses(&c.statement); if s.is_some() { return s; } @@ -567,12 +564,9 @@ fn job_is_simple_block(&self, job: &ast::JobPipeline) -> bool { let no_redirs = |list: &ast::ArgumentOrRedirectionList| !list.iter().any(|val| val.is_redirection()); - let Some(statement) = job.statement3() else { - return true; - }; // Check if we're a block statement with redirections. We do it this obnoxious way to preserve // type safety (in case we add more specific statement types). - match &statement.contents { + match &job.statement.contents { StatementVariant::BlockStatement(stmt) => no_redirs(&stmt.args_or_redirs), StatementVariant::BraceStatement(stmt) => no_redirs(&stmt.args_or_redirs), StatementVariant::SwitchStatement(stmt) => no_redirs(&stmt.args_or_redirs), @@ -614,17 +608,12 @@ fn apply_variable_assignments( ctx: &OperationContext<'_>, mut proc: Option<&mut Process>, variable_assignment_list: &ast::VariableAssignmentList, - block: Option<&mut Option>, + block: &mut Option, ) -> EndExecutionReason { if variable_assignment_list.is_empty() { return EndExecutionReason::ok; } - // FIXME EnvMode::USER - let mut flags = EnvMode::default(); - if let Some(block) = block { - *block = Some(ctx.parser().push_block(Block::variable_assignment_block())); - flags = EnvMode::LOCAL | EnvMode::EXPORT; - } + *block = Some(ctx.parser().push_block(Block::variable_assignment_block())); for variable_assignment in variable_assignment_list { let source = self.node_source(&**variable_assignment); let equals_pos = variable_assignment_equals_pos(source).unwrap(); @@ -664,7 +653,8 @@ fn apply_variable_assignments( vals.clone(), )); } - ctx.parser().set_var_and_fire(variable_name, flags, vals); + ctx.parser() + .set_var_and_fire(variable_name, EnvMode::LOCAL | EnvMode::EXPORT, vals); } EndExecutionReason::ok } @@ -675,20 +665,17 @@ fn populate_job_process( ctx: &OperationContext<'_>, job: &mut Job, proc: &mut Process, - variable_statement: &ast::VariableStatement, + statement: &ast::Statement, + variable_assignments: &ast::VariableAssignmentList, ) -> EndExecutionReason { - let mut block = variable_statement.statement.is_some().then_some(None); - let result = self.apply_variable_assignments( - ctx, - Some(proc), - &variable_statement.variables, - block.as_mut(), - ); - let Some(statement) = variable_statement.statement.as_ref() else { - return result; - }; + // Get the "specific statement" which is boolean / block / if / switch / decorated. + let specific_statement = &statement.contents; + + let mut block = None; + let result = + self.apply_variable_assignments(ctx, Some(proc), variable_assignments, &mut block); let _scope = ScopeGuard::new((), |()| { - if let Some(Some(block)) = block { + if let Some(block) = block { ctx.parser().pop_block(block); } }); @@ -696,8 +683,6 @@ fn populate_job_process( return result; } - // Get the "specific statement" which is boolean / block / if / switch / decorated. - let specific_statement = &statement.contents; match &specific_statement { StatementVariant::NotStatement(not_statement) => { self.populate_not_process(ctx, job, proc, not_statement) @@ -726,7 +711,13 @@ fn populate_not_process( let mut flags = job.mut_flags(); flags.negate = !flags.negate; } - self.populate_job_process(ctx, job, proc, ¬_statement.negated_statement) + self.populate_job_process( + ctx, + job, + proc, + ¬_statement.contents, + ¬_statement.variables, + ) } /// Creates a 'normal' (non-block) process. @@ -1592,24 +1583,16 @@ fn run_1_job( // However, if there are no redirections, then we can just jump into the block directly, which // is significantly faster. if self.job_is_simple_block(job_node) { - let variable_statement = &job_node.statement2; - let mut block = variable_statement.statement.is_some().then_some(None); - let mut result = self.apply_variable_assignments( - ctx, - None, - &variable_statement.variables, - block.as_mut(), - ); - let Some(statement) = variable_statement.statement.as_ref() else { - return result; - }; + let mut block = None; + let mut result = + self.apply_variable_assignments(ctx, None, &job_node.variables, &mut block); let _scope = ScopeGuard::new((), |()| { - if let Some(Some(block)) = block { + if let Some(block) = block { ctx.parser().pop_block(block); } }); - let specific_statement = &statement.contents; + let specific_statement = &job_node.statement.contents; assert!(specific_statement_type_is_redirectable_block( specific_statement )); @@ -1836,7 +1819,13 @@ fn populate_job_from_job_node( // Create processes. Each one may fail. let mut processes = ProcessList::new(); processes.push(Box::new(Process::new())); - let mut result = self.populate_job_process(ctx, j, &mut processes[0], &job_node.statement2); + let mut result = self.populate_job_process( + ctx, + j, + &mut processes[0], + &job_node.statement, + &job_node.variables, + ); // Construct process_ts for job continuations (pipelines). for jc in &job_node.continuation { @@ -1870,8 +1859,13 @@ fn populate_job_from_job_node( // Store the new process (and maybe with an error). processes.push(Box::new(Process::new())); - result = - self.populate_job_process(ctx, j, processes.last_mut().unwrap(), &jc.statement2); + result = self.populate_job_process( + ctx, + j, + processes.last_mut().unwrap(), + &jc.statement, + &jc.variables, + ); } // Inform our processes of who is first and last @@ -2022,29 +2016,18 @@ fn job_node_wants_timing(job_node: &ast::JobPipeline) -> bool { if ns.time.is_some() { return true; } - stat = match ns.negated_statement.statement.as_ref() { - Some(s) => s, - None => return false, - }; + stat = &ns.contents; } _ => return false, } }; // Do we have a 'not time ...' anywhere in our pipeline? - if job_node - .statement3() - .map(is_timed_not_statement) - .unwrap_or_default() - { + if is_timed_not_statement(&job_node.statement) { return true; } for jc in &job_node.continuation { - if jc - .statement() - .map(is_timed_not_statement) - .unwrap_or_default() - { + if is_timed_not_statement(&jc.statement) { return true; } } diff --git a/src/parse_tree.rs b/src/parse_tree.rs index 33598d34b..6922b5402 100644 --- a/src/parse_tree.rs +++ b/src/parse_tree.rs @@ -28,7 +28,7 @@ pub struct ParseToken { /// Hackish: if TOK_END, whether the source is a newline. pub is_newline: bool, // Hackish: whether this token is a string like FOO=bar - pub is_variable_assignment: bool, + pub may_be_variable_assignment: bool, /// If this is a tokenizer error, that error. pub tok_error: TokenizerError, source_start: SourceOffset, @@ -43,7 +43,7 @@ pub fn new(typ: ParseTokenType) -> Self { has_dash_prefix: false, is_help_argument: false, is_newline: false, - is_variable_assignment: false, + may_be_variable_assignment: false, tok_error: TokenizerError::none, source_start: SOURCE_OFFSET_INVALID.try_into().unwrap(), source_length: 0, diff --git a/src/parse_util.rs b/src/parse_util.rs index c306d31ac..deb0b7ddd 100644 --- a/src/parse_util.rs +++ b/src/parse_util.rs @@ -664,7 +664,6 @@ fn get_first_arg(list: &ast::ArgumentOrRedirectionList) -> Option<&ast::Argument /// Given a wide character immediately after a dollar sign, return the appropriate error message. /// For example, if wc is @, then the variable name was $@ and we suggest $argv. fn error_for_character(c: char) -> WString { - // TODO match c { '?' => wgettext!(ERROR_NOT_STATUS).to_owned(), '#' => wgettext!(ERROR_NOT_ARGV_COUNT).to_owned(), @@ -1201,12 +1200,7 @@ pub fn parse_util_detect_errors_in_ast( if let Some(jc) = node.as_job_continuation() { // Somewhat clumsy way of checking for a statement without source in a pipeline. // See if our pipe has source but our statement does not. - if jc.pipe.has_source() - && jc - .statement() - .and_then(|stmt| stmt.try_source_range()) - .is_none() - { + if jc.pipe.has_source() && jc.statement.try_source_range().is_none() { has_unclosed_pipe = true; } } else if let Some(job_conjunction) = node.as_job_conjunction() { @@ -1407,21 +1401,7 @@ pub fn parse_util_detect_errors_in_argument( continue; } let next_char = unesc.get(idx + 1).copied().unwrap_or('\0'); - let prev_char = idx - .checked_sub(1) - .and_then(|i| unesc.get(i).copied()) - .unwrap_or('\0'); - if (![ - VARIABLE_EXPAND, - VARIABLE_EXPAND_SINGLE, - '$', - '(', - '?', - '#', - '@', - ] - .contains(&next_char) - && ![VARIABLE_EXPAND, VARIABLE_EXPAND_SINGLE, '$'].contains(&prev_char)) + if ![VARIABLE_EXPAND, VARIABLE_EXPAND_SINGLE, '('].contains(&next_char) && !valid_var_name_char(next_char) { err = ParserTestErrorBits::ERROR; @@ -1632,8 +1612,7 @@ fn detect_errors_in_decorated_statement( // Check our pipeline position. let pipe_pos = if job.continuation.is_empty() { PipelinePosition::none - } else if job.statement3().unwrap().pointer_eq(st) { - // TODO(posix_mode) unwrap + } else if job.statement.pointer_eq(st) { PipelinePosition::first } else { PipelinePosition::subsequent @@ -1914,8 +1893,6 @@ pub fn parse_util_expand_variable_error( '\0' => { append_syntax_error!(errors, global_dollar_pos, 1, ERROR_NO_VAR_NAME); } - // TODO - '?' | VARIABLE_EXPAND | VARIABLE_EXPAND_SINGLE | '$' | '#' | '@' => return, _ => { let mut token_stop_char = char_after_dollar; // Unescape (see issue #50). diff --git a/src/tests/key.rs b/src/tests/key.rs index 4b8350759..e99ee575a 100644 --- a/src/tests/key.rs +++ b/src/tests/key.rs @@ -9,7 +9,6 @@ fn test_parse_key() { ); assert_eq!(parse_keys(L!("\x1b")), Ok(vec![Key::from_raw(key::Escape)])); assert_eq!(parse_keys(L!("ctrl-a")), Ok(vec![ctrl('a')])); - assert_eq!(parse_keys(L!("c-a")), Ok(vec![ctrl('a')])); assert_eq!(parse_keys(L!("\x01")), Ok(vec![ctrl('a')])); assert!(parse_keys(L!("f0")).is_err()); assert_eq!( diff --git a/src/tests/parse_util.rs b/src/tests/parse_util.rs index 6dedd4038..b41a94649 100644 --- a/src/tests/parse_util.rs +++ b/src/tests/parse_util.rs @@ -3,7 +3,8 @@ use crate::common::EscapeFlags; use crate::parse_constants::{ ERROR_BAD_VAR_CHAR1, ERROR_BRACKETED_VARIABLE1, ERROR_BRACKETED_VARIABLE_QUOTED1, - ERROR_NOT_ARGV_STAR, ERROR_NO_VAR_NAME, + ERROR_NOT_ARGV_AT, ERROR_NOT_ARGV_COUNT, ERROR_NOT_ARGV_STAR, ERROR_NOT_PID, ERROR_NOT_STATUS, + ERROR_NO_VAR_NAME, }; use crate::parse_util::{ parse_util_cmdsubst_extent, parse_util_compute_indents, parse_util_detect_errors, @@ -74,6 +75,10 @@ macro_rules! validate { validate!("echo foo${a}bar", ERROR_BRACKETED_VARIABLE1); validate!("echo foo\"${a}\"bar", ERROR_BRACKETED_VARIABLE_QUOTED1); validate!("echo foo\"${\"bar", ERROR_BAD_VAR_CHAR1); + validate!("echo $?", ERROR_NOT_STATUS); + validate!("echo $$", ERROR_NOT_PID); + validate!("echo $#", ERROR_NOT_ARGV_COUNT); + validate!("echo $@", ERROR_NOT_ARGV_AT); validate!("echo $*", ERROR_NOT_ARGV_STAR); validate!("echo $", ERROR_NO_VAR_NAME); validate!("echo foo\"$\"bar", ERROR_NO_VAR_NAME); diff --git a/src/tests/parser.rs b/src/tests/parser.rs index 5c0f0a9af..0b89806f6 100644 --- a/src/tests/parser.rs +++ b/src/tests/parser.rs @@ -41,11 +41,6 @@ fn detect_argument_errors(src: &str) -> Result<(), ParserTestErrorBits> { parse_util_detect_errors_in_argument(first_arg, first_arg.source(&src), &mut errors) } - assert!( - detect_errors!("true && ") == Err(ParserTestErrorBits::INCOMPLETE), - "unterminated conjunction not reported properly" - ); - // Testing block nesting assert!( detect_errors!("if; end").is_err(), @@ -198,7 +193,14 @@ fn detect_argument_errors(src: &str) -> Result<(), ParserTestErrorBits> { ); assert!( - detect_argument_errors("foo$%") + detect_argument_errors("foo$$") + .unwrap_err() + .contains(ParserTestErrorBits::ERROR), + "Bad variable expansion not reported as error" + ); + + assert!( + detect_argument_errors("foo$@") .unwrap_err() .contains(ParserTestErrorBits::ERROR), "Bad variable expansion not reported as error" @@ -552,9 +554,13 @@ fn test_new_parser_ad_hoc() { // The bug was that "a=" would produce an error but not be consumed, // leading to an infinite loop. + // By itself it should produce an error. let ast = Ast::parse(L!("a="), ParseTreeFlags::default(), None); - assert!(!ast.errored()); + assert!(ast.errored()); + // If we are leaving things unterminated, this should not produce an error. + // i.e. when typing "a=" at the command line, it should be treated as valid + // because we don't want to color it as an error. let ast = Ast::parse(L!("a="), ParseTreeFlags::LEAVE_UNTERMINATED, None); assert!(!ast.errored()); @@ -618,6 +624,8 @@ macro_rules! validate { validate!("begin ; }", ParseErrorCode::unbalancing_brace); validate!("true | and", ParseErrorCode::andor_in_pipeline); + + validate!("a=", ParseErrorCode::bare_variable_assignment); } #[test] diff --git a/tests/checks/commandline.fish b/tests/checks/commandline.fish index 642ce9226..6562d5873 100644 --- a/tests/checks/commandline.fish +++ b/tests/checks/commandline.fish @@ -16,6 +16,7 @@ or echo Invalid $status commandline --input 'echo $$' --is-valid or echo Invalid $status +# CHECK: Invalid 1 commandline --help &>/dev/null echo Help $status diff --git a/tests/checks/expansion.fish b/tests/checks/expansion.fish index ad6e51f8a..713fc509c 100644 --- a/tests/checks/expansion.fish +++ b/tests/checks/expansion.fish @@ -324,8 +324,11 @@ $fish -c 'echo {}}' #CHECKERR: fish: Unexpected '}' for unopened brace #CHECKERR: echo {}} #CHECKERR: ^ -printf '<%s>\n' ($fish -c 'command (echo asd)' 2>&1) -# CHECK: +printf '<%s>\n' ($fish -c 'command (asd)' 2>&1) +#CHECK: +#CHECK: +#CHECK: < ^~~~^> +true printf '<%s>\n' ($fish -c 'echo "$abc["' 2>&1) #CHECK: @@ -334,7 +337,7 @@ printf '<%s>\n' ($fish -c 'echo "$abc["' 2>&1) set -l pager command less echo foo | $pager -#CHECKERR: {{.*}}checks/expansion.fish (line {{\d+}}): The expanded command is a keyword. +#CHECKERR: {{.*}}checks/expansion.fish (line 339): The expanded command is a keyword. #CHECKERR: echo foo | $pager #CHECKERR: ^~~~~^ diff --git a/tests/checks/indent.fish b/tests/checks/indent.fish index 31bf67246..89ca9d4f9 100644 --- a/tests/checks/indent.fish +++ b/tests/checks/indent.fish @@ -403,7 +403,8 @@ function hello_continuations echo "\ a=1 \\ - a=2 echo" | $fish_indent --check # FIXME + a=2 \\ + echo" | $fish_indent --check echo $status #CHECK: 0 echo "\ @@ -496,12 +497,12 @@ echo $status #CHECK: 0 echo 'PATH={$PATH[echo " "' | $fish_indent --ansi # CHECK: PATH={$PATH[echo " " -echo a \> | $fish_indent # TODO(posix_mode) +echo a\> | $fish_indent # CHECK: a > echo a\<\) | $fish_indent # CHECK: a < ) -echo b\|\{ | $fish_indent # TODO(posix_mode) +echo b\|\{ | $fish_indent # CHECK: b | { echo "\'\\\\\x00\'" | string unescape | $fish_indent | string escape diff --git a/tests/checks/invocation.fish b/tests/checks/invocation.fish index 0b728561f..623bdf6ac 100644 --- a/tests/checks/invocation.fish +++ b/tests/checks/invocation.fish @@ -91,20 +91,20 @@ $fish --no-config -c 'echo notprinted; echo foo | exec true; echo banana' # CHECKERR: ^~~~~~~~^ # Running multiple command lists continues even if one has a syntax error. -$fish --no-config -c 'echo $% oh no syntax error' -c 'echo this works' +$fish --no-config -c 'echo $$ oh no syntax error' -c 'echo this works' # CHECK: this works -# CHECKERR: fish: $% is not a valid variable in fish. -# CHECKERR: echo $% oh no syntax error +# CHECKERR: fish: $$ is not the pid. In fish, please use $fish_pid. +# CHECKERR: echo $$ oh no syntax error # CHECKERR: ^ $fish --no-config . # CHECKERR: error: Unable to read input file: Is a directory # CHECKERR: warning: Error while reading file . -$fish --no-config -c 'echo notprinted; echo foo; echo $%' -# CHECKERR: fish: $% is not a valid variable in fish. -# CHECKERR: echo notprinted; echo foo; echo $% -# CHECKERR: ^ +$fish --no-config -c 'echo notprinted; echo foo; a=b' +# CHECKERR: fish: Unsupported use of '='. In fish, please use 'set a b'. +# CHECKERR: echo notprinted; echo foo; a=b +# CHECKERR: ^~^ $fish --no-config -c 'echo notprinted | and true' # CHECKERR: fish: The 'and' command can not be used in a pipeline diff --git a/tests/checks/syntax-error-location.fish b/tests/checks/syntax-error-location.fish index 300a954f8..14d0df8d2 100644 --- a/tests/checks/syntax-error-location.fish +++ b/tests/checks/syntax-error-location.fish @@ -17,6 +17,18 @@ echo 'true | time false' | $fish 2>| string replace -r '(.*)' '<$1>' # CHECK: < ^~~~~~~~~^> +echo ' + +FOO=BAR (true one) +(true two) + +# more things +' | $fish 2>| string replace -r '(.*)' '<$1>' + +# CHECK: +# CHECK: +# CHECK: < ^~~~~~~~~^> + $fish -c 'echo "unfinished "(subshell' 2>| string replace -r '.*' '<$0>' # CHECK: # CHECK: diff --git a/tests/checks/variable-assignment.fish b/tests/checks/variable-assignment.fish index 67233de4c..d5fb6b6ef 100644 --- a/tests/checks/variable-assignment.fish +++ b/tests/checks/variable-assignment.fish @@ -85,19 +85,18 @@ complete -C'a=b envxalias ' # CHECK: arg # Eval invalid grammar to allow fish to parse this file -a=(echo b1) -echo a is $a -# CHECK: a is b1 - -# FIXME -# : | a=b2 -# echo a is $a -# # CHECK: echo a is b2 - -# TODO(posix_mode) -# not a=b3 -# echo a is $a -# # CHECK: echo a is b3 +eval 'a=(echo b)' +# CHECKERR: {{.*}}: Unsupported use of '='. In fish, please use 'set a (echo b)'. +# CHECKERR: a=(echo b) +# CHECKERR: ^~~~~~~~~^ +eval ': | a=b' +# CHECKERR: {{.*}}: Unsupported use of '='. In fish, please use 'set a b'. +# CHECKERR: : | a=b +# CHECKERR: ^~^ +eval 'not a=b' +# CHECKERR: {{.*}}: Unsupported use of '='. In fish, please use 'set a b'. +# CHECKERR: not a=b +# CHECKERR: ^~^ complete -c foo -xa '$a' a=b complete -C'foo '