Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(commands): trim end of pipe-like output #10952

Merged
merged 1 commit into from
Jul 13, 2024

Conversation

RoloEdits
Copy link
Contributor

Current pipe-like commands, pipe, insert-output, and append-output, return directly what stdout had, which usually includes a newline. This can limit its efficiency in the editor as when someone would want to run the same command on many selections, the additional whitespace can become cumbersome to manually trim.

This PR trims all whitespace from the end of the output. This was mainly chosen do to the simplicity of the change, but even when testing an iteration where only a single newline was removed, it felt more inline with expectations and so was chosen for the final change.

pipe:exa

base16_theme.toml
book
Cargo.lock
Cargo.toml
CHANGELOG.md
contrib
default.nix
docs
Files
flake.lock
flake.nix
grammars.nix
helix-core
helix-dap
helix-event
helix-loader
helix-lsp
helix-parsec
helix-stdx
helix-term
helix-tui
helix-vcs
helix-view
languages.toml
LICENSE
logo.svg
logo_dark.svg
logo_light.svg
README.md
runtime
rust-toolchain.toml
rustfmt.toml
screenshot.png
shell.nix
target
theme.toml
xtask
-       

Closes: #10912

@RoloEdits
Copy link
Contributor Author

RoloEdits commented Jun 14, 2024

I'm unsure if I implemented the change in full completeness, as the exiting logic is kind of hard to understand. When trying the commands in the editor, this worked fine:

        let output = if let Some(output) = shell_output.as_ref() {
            output.clone()
        } else {
            let fragment = range.slice(text);
            match shell_impl(shell, cmd, pipe.then(|| fragment.into())) {
                Ok(result) => {
                    if !pipe {
                        shell_output = Some(result.clone());
                    }
-                    result
+                    Tendril::from(result.trim_end())
                }
                Err(err) => {
                    cx.editor.set_error(err.to_string());
                    return;
                }
            }
        };

But the test failed.

When adding this, the test passed:

        let output = if let Some(output) = shell_output.as_ref() {
-           output.clone()
+           Tendril::from(output.trim_end())
        } else {
            let fragment = range.slice(text);
            match shell_impl(shell, cmd, pipe.then(|| fragment.into())) {
                Ok(result) => {
                    if !pipe {
                        shell_output = Some(result.clone());
                    }
                    result
                    Tendril::from(result.trim_end())
                }
                Err(err) => {
                    cx.editor.set_error(err.to_string());
                    return;
                }
            }
        };

But still not sure if shell_output = Some(result.clone()); needs to be done as well?

edit:

Changed to this and it worked with tests passing:

        let output = if let Some(output) = shell_output.as_ref() {
            output.clone()
        } else {
            let fragment = range.slice(text);
            match shell_impl(shell, cmd, pipe.then(|| fragment.into())) {
                Ok(result) => {
+                    let result = Tendril::from(result.trim_end());
                    if !pipe {
                        shell_output = Some(result.clone());
                    }
                    result
                }
                Err(err) => {
                    cx.editor.set_error(err.to_string());
                    return;
                }
            }
        };

@the-mikedavis the-mikedavis added the A-command Area: Commands label Jun 14, 2024
@archseer archseer merged commit fd7b1a3 into helix-editor:master Jul 13, 2024
6 checks passed
@RoloEdits RoloEdits deleted the pipe-trim-end branch July 13, 2024 12:12
mxxntype pushed a commit to mxxntype/helix that referenced this pull request Aug 14, 2024
kyruzic pushed a commit to kyruzic/helix that referenced this pull request Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-command Area: Commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trim final newline from :pipe output
3 participants