search

microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
162.66k stars 28.68k forks source link

Terminal: Copy on selection + new highlight in 1.68 copies previous term on CMD+F #151902

Open vfonic opened 2 years ago

vfonic commented 2 years ago

Does this issue occur when all extensions are disabled?: Yes/No

Steps to Reproduce:

  1. In settings.json enable: "terminal.integrated.copyOnSelection": true,
  2. Open integrated terminal
  3. Search for some term that exists in the terminal (that will be highlighted/selected)
  4. Focus on editor and type + copy some chunk of text
  5. Focus on terminal, press CMD+F and paste what you just copied

Expected:

The pasted term is the term you just copied

Actual:

The pasted term is the term you last searched for (in step 3).

Why this happens?

Because:

  1. When you press CMD+F in terminal, the terminal has pre-existing term typed in the search box (from your first search)
  2. Terminal uses this term to highlight all occurrences of such term in the terminal
  3. Highlighting the terms makes the terminal also copy this term to clipboard due to "terminal.integrated.copyOnSelection": true,
VSCodeTriageBot commented 2 years ago

Thanks for creating this issue! It looks like you may be using an old version of VS Code, the latest stable release is 1.68.0. Please try upgrading to the latest version and checking whether this issue remains.

Happy Coding!

VSCodeTriageBot commented 2 years ago

The described behavior is how it is expected to work. If you disagree, please explain what is expected and what is not in more detail. See also our issue reporting guidelines.

Happy Coding!

vfonic commented 2 years ago

@meganrogge I believe you might have misunderstood my issue. When pressing CMD+F, the old term is added to the clipboard and the old term is highlighted and searched for, not the new term that the user wanted to search for.

For example:

  1. You search for "pasta" in terminal.
  2. You copy word "donut"
  3. Press CMD+F
  4. Press CMD+V (to paste)
  5. The pasted word in search box would be "pasta"

Please try to follow the repro steps to see how this bug happens.

joelhock commented 2 years ago

@meganrogge this has been happening to me, too, and the repro that @vfonic describes is spot on. can you reopen this?

meganrogge commented 2 years ago

I still think that's as designed because the find widget uses the current terminal selection as the input for find

joelhock commented 2 years ago

@meganrogge this isn't about using the current selection as the input for find, it's about any matches to what's in the find box overwriting the clipboard entry. this certainly wasn't the behavior in previous versions. here's a video to illustrate:

https://user-images.githubusercontent.com/7648957/173935510-bef58bf9-c0b9-49b8-81f6-c5bd8c364612.mov

vfonic commented 2 years ago

@meganrogge did you try following the repro steps?

Tyriar commented 2 years ago

Thanks for the gif, yes we misunderstood. Re-searching after content is added to the terminal should not copy the text, only when the find is explicitly triggered.

meganrogge commented 2 years ago

this will be pretty involved currently - think we should fix it when we let each terminal have it's own find widget, #145865 because we'd need to do something like this and currently the instance doesn't have access to the find widget (it's owned by the tabs view)

Screen Shot 2022-06-27 at 1 38 59 PM
vfonic commented 2 years ago

Oh :/

I've downgraded in the meantime as this is a huge issue for my workflow.

I fully support terminal having its own search widget, because I'm also using some other customization that depends on "terminal being in focus" and that doesn't work when searching in terminal.

Thank you for looking into this!

joelhock commented 2 years ago

My two cents is that even when find is explicitly triggered, a found selection's text should not be copied to the clipboard--maybe that's an easier fix?

movy commented 2 years ago

Thanks for creating this issue, I've been bugged by "accidental" clipboard contamination with some random stuff and finally figured out where it's coming from. Hope it can be fixed soon.

krasi-georgiev commented 1 year ago

What is the current status? Abandoned or added to a milestone?

krasi-georgiev commented 1 year ago

still a pending bug in

Version: 1.79.0-insider Commit: f6be5461f8bc69013a605f5baea834651c6589fb Date: 2023-05-20T01:20:25.075Z Electron: 22.5.2 Chromium: 108.0.5359.215 Node.js: 16.17.1 V8: 10.8.168.25-electron.0 OS: Linux x64 5.15.0-72-generic

movy commented 1 year ago

and extremely annoying also, took me like 10 attempts to figure out why the terminal keeps searching for something I did not put into the search field.

movy commented 1 year ago

I think the problem lies in erroneous 'copy on selection' behaviour, when EACH focus on the terminal with active text selection copies the selected text again and again. The only time copying should be performed is when the selection ends (i.e. mouse up or whatever event is responsible for this), not each time terminal pane gets focus.

movy commented 1 year ago

Moreover! This bug extends much further:

My clipboard history after typing in terminal search field:

image

See video below for explanation:

https://github.com/microsoft/vscode/assets/4336560/f30f214a-9f89-4b26-b130-1ae0f204cde6

krasi-georgiev commented 1 year ago

Can the maintainers update us on when can this be scheduled for a fix.

krasi-georgiev commented 8 months ago

unfortunately, the bug is still present.

When I leave the search box open and the terminal is set to copy on selection it frequently hijacks the clipboard and places the text in the search box in the clipboard.

Version: 1.86.0-insider Commit: df8db3a75a49b85c9636530a3557cdbc639f7bdc Date: 2024-01-01T05:35:59.858Z Electron: 27.1.3 ElectronBuildId: 25612240 Chromium: 118.0.5993.159 Node.js: 18.17.1 V8: 11.8.172.18-electron.0 OS: Linux x64 5.15.0-91-generic

movy commented 8 months ago

+1 for the most recent comment, noticed it yesterday as well, very frustrating

Tyriar commented 7 months ago

@krasi-georgiev @movy any more info on the repro? I tried opening search, clicking between editor, terminal and search and couldn't get it to copy the text in the search box?

joelhock commented 7 months ago

this shows a repro:

https://github.com/microsoft/vscode/assets/7648957/c8e510b2-5bfb-48a8-8aff-06e957066c37

Tyriar commented 7 months ago

Thanks I can repro, I think what's happening is the the find is doing another search when the content changes and since the find widget isn't active it's getting copied to the clipboard.

vfonic commented 7 months ago

@Tyriar

any more info on the repro?

Hey @Tyriar can you please tell me which of the repro steps I posted originally 2 years ago is confusing to you?

  1. In settings.json enable: "terminal.integrated.copyOnSelection": true,
  2. Open integrated terminal
  3. Search for some term that exists in the terminal (that will be highlighted/selected)
  4. Focus on editor and type + copy some chunk of text
  5. Focus on terminal, press CMD+F and paste what you just copied

Here's the screen recording showing exactly these repro steps causing the bug: https://github.com/microsoft/vscode/assets/67437/ff749c31-30b1-4564-be2a-6e9a2daaa555

BTW I tested on VS Code 1.85.2 stable, but I've seen that someone else already tested on the latest insiders build and that the bug is still present.

Tyriar commented 7 months ago

@vfonic that bug was fixed in 1.86, @joelhock helped find the edge case that was missed.

vfonic commented 7 months ago

@Tyriar indeed! I just tested it on the VS Code Insiders and the bug I mentioned with my repro steps is no longer happening! Thank you! 🙏

Pictor13 commented 7 months ago

Finally! I was also struggling a lot in my workflow with this regression. Thanks a lot for fixing it! :pray: (and tnx @vfonic for reporting and providing repro instruction 🙂 )