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

bracketed paste only works the first time #648

Closed
sigoden opened this issue Oct 17, 2023 · 12 comments
Closed

bracketed paste only works the first time #648

sigoden opened this issue Oct 17, 2023 · 12 comments
Labels
bug Something isn't working

Comments

@sigoden
Copy link
Contributor

sigoden commented Oct 17, 2023

Platform Ubuntu 22.04
Terminal software gnome-terminal

bracketed paste only works the first time.

This bug does not exist on v0.19.1 but does on v0.25.0

Steps to reproduce

  1. run cargo run --example demo
  2. paste multi-line content, successful, press ctrl+c, paste multi-line content again, it failed

Screenshots/Screencaptures

reedline

@sigoden sigoden added the bug Something isn't working label Oct 17, 2023
@fdncred
Copy link
Collaborator

fdncred commented Oct 17, 2023

I can reproduce this on Ubuntu in WSL. I wonder if it's a bug in crossterm or if we've just implemented it wrong?

@sigoden
Copy link
Contributor Author

sigoden commented Oct 17, 2023

@fdncred
Copy link
Collaborator

fdncred commented Oct 17, 2023

Ya, that first link could be the issue. I'm not sure why we'd disable it on every read_line? Seems like we should check to see if it is meant to be enabled or not before automatically always disabling it?

Just playing around with the demo, I noticed that if I enable it on every loop, it works fine.

    loop {
        let res = line_editor.enable_bracketed_paste();
        let bracketed_paste_enabled = res.is_ok();
        if !bracketed_paste_enabled {
            println!("Warn: failed to enable bracketed paste mode: {res:?}");
        }

        let sig = line_editor.read_line(&prompt);

Lemme try to take that engine part out while I'm playing around and see if that fixes it.

@fdncred
Copy link
Collaborator

fdncred commented Oct 17, 2023

ya, commenting out these lines makes it work too.

    pub fn read_line(&mut self, prompt: &dyn Prompt) -> Result<Signal> {
        terminal::enable_raw_mode()?;

        let result = self.read_line_helper(prompt);

        // #[cfg(not(target_os = "windows"))]
        // self.disable_bracketed_paste()?;

        if self.use_kitty_protocol {
            let _ = execute!(io::stdout(), event::PopKeyboardEnhancementFlags);
        }

        terminal::disable_raw_mode()?;
        result
    }

I wonder if we should have something like how the kitty protocol is checked? e.g. if self.use_bracketed_paste { ... }

@amtoine
Copy link
Member

amtoine commented Oct 17, 2023

@sigoden
is this related to nushell/nushell#9944 as well? 😋

@LevitatingBusinessMan
Copy link
Contributor

Huh, so this is why I kept thinking that bracketed paste wasn't working yet when others said it did. The feature was literally disabled on Linux...

@fdncred
Copy link
Collaborator

fdncred commented Nov 3, 2023

@LevitatingBusinessMan Feel free to create a PR that works better. It apparently works the first time you paste but not after that.

@LevitatingBusinessMan
Copy link
Contributor

LevitatingBusinessMan commented Nov 4, 2023

The fix might just need to be to revert #598. Because a single application is being buggy doesn't mean you should disable an entire feature for a platform. I have no idea how that even got merged, the original PR it originates from re-set the bracketed paste but now it just disables it. Seemingly no-one cared to actually review that PR before merging.

@fdncred
Copy link
Collaborator

fdncred commented Nov 4, 2023

@LevitatingBusinessMan In case you're not aware, your comments here and in #598 are rude and disrespectful. Reedline and Nushell is written and maintained by a handful of volunteers. We're all human and have different technical backgrounds, we make mistakes sometimes, and we have feelings. We're always up for constructive criticism but you're not being constructive. Please be mindful of this going forward. Thank you.

@LevitatingBusinessMan
Copy link
Contributor

@fdncred This project has thousands of daily downloads on crates.io. I'd argue a project of this size comes with certain responsibilities. I understand how bugs can be merged, it happens to every project. But in this case a feature was blatantly disabled on a platform and it was stated as a bugfix in the changelog.

This could've easily been prevented if the reviewer had actually reviewed the change (let alone tested it). Or if the merge had happened on the nushell side as was proposed originally.

@fdncred
Copy link
Collaborator

fdncred commented Nov 4, 2023

What I was objecting to was your tone of dismissiveness and greater-than-thou attitude that was coming across. Again, maybe you didn't mean to act that way, but that's how it was coming across. I like to give people the benefit of the doubt because it's so hard to read emotion and intent in text sometimes. We all try hard, and everyone does their best. We have an excellent core-team. We don't appreciate people being rude and disrespectful to us and trying to bully us into making a change or undo a change.

Please feel free to submit a PR that you believe fixes the problem, otherwise let's please move on.

@LevitatingBusinessMan
Copy link
Contributor

Fixed in #657

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants