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

Opening file with source:line breaks Cmd+D "Add Next Occurance" #79460

Closed
bbugh opened this issue Aug 19, 2019 · 29 comments
Closed

Opening file with source:line breaks Cmd+D "Add Next Occurance" #79460

bbugh opened this issue Aug 19, 2019 · 29 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug *duplicate Issue identified as a duplicate of another issue(s) keybindings VS Code keybinding issues macos Issues with VS Code on MAC/OS X

Comments

@bbugh
Copy link

bbugh commented Aug 19, 2019

  • VSCode Version: 1.37.1
  • OS Version: Mac 10.12.6

Issue

Here's a weird one for you! This is 100% reproducible, and only appeared recently.

Whenever I open Visual Studio Code using line numbers (from iTerm2 shortcut or others), the Cmd+D shortcut key to select other instances doesn't work until the VSCode window loses and regains focus.

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

Steps to Reproduce:

Part 1

  1. Have iTerm2
  2. Cmd + Click on a file name
  3. Once VSCode opens, Cmd+D on some text that should have another match
  4. Notice that the "Selection" menu item at the top flashes as activated, but no new matching selections are selected

Part 2

  1. Click around in VSCode, in the same editor, other editors, etc.
  2. Press Cmd+D
  3. Notice that the "Selection" menu item at the top flashes as activated, but no new matching selections are selected

Part 3

  1. Cmd+Tab to another window
  2. Cmd+Tab back to VSCode
  3. Notice that the Cmd+D key shortcut is now working as expected.
@vscodebot
Copy link

vscodebot bot commented Aug 19, 2019

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

@RMacfarlane RMacfarlane added the keybindings VS Code keybinding issues label Aug 19, 2019
@alexdima alexdima added the macos Issues with VS Code on MAC/OS X label Aug 20, 2019
@alexdima
Copy link
Member

alexdima commented Aug 20, 2019

@bpasero Could we have some bug where we don't know the current focused window at the beginning and then we don't send the command invocation?

Note this gets fixed once focusing out and focusing in ...

Possibly related -- #71553

@bpasero
Copy link
Member

bpasero commented Aug 20, 2019

@bpasero Could we have some bug where we don't know the current focused window at the beginning and then we don't send the command invocation?

@alexandrudima how can we detect a keypress in the first place if the window is not focused? Are you maybe thinking of macOS where all keys go through the top level menu and not the active window? But this seems to be Windows...

@bbugh
Copy link
Author

bbugh commented Aug 20, 2019

Hi, I clarified in the first message that it's Mac OS. I originally pasted in the version number but didn't specify that it was Mac, sorry about that.

@bpasero bpasero assigned sbatten and unassigned bpasero Aug 20, 2019
@bpasero
Copy link
Member

bpasero commented Aug 20, 2019

Got it, moving to @sbatten who owns menu interactions.

@bbugh
Copy link
Author

bbugh commented Aug 20, 2019

#71553 does seem possibly related, but opening files from the command line (for me) does not exhibit the behavior I described in the issue. I am able to Cmd+D fine in that scenario.

I tried a couple of other ideas:

  • code Gemfile -> Cmd+D works fine
  • Cmd+Left Click on the Gemfile -> Cmd+D works fine
  • Cmd+Left Click on "Gemfile:97" -> Cmd+D does not work.

It appears to be related to however VSCode is opening files and going to a specific line number.

@alexdima
Copy link
Member

@bpasero The menu bar calls IWindowService.sendToFocused -- here it is. Since the menu bar is flashing, it means it is eating up the keybinding so the keybinding does not make it to the renderer and we are relying on vscode:runAction to be sent from the main process to the window...

My guess would be that the IWindowService.sendToFocused is not working sometimes, when launched in very specific ways. I could reproduce #71553 on my mac with #71553 (comment).

@bpasero
Copy link
Member

bpasero commented Aug 20, 2019

@alexandrudima I tried to setup #71553 (comment) and my menu bar does not flash and Cmd+D works to select words, here is proof:

before

I used a fresh user data dir to rule out the possibility of a setting having an impact. What are you doing different?

@bpasero bpasero assigned bpasero and unassigned sbatten Aug 20, 2019
@bpasero
Copy link
Member

bpasero commented Aug 20, 2019

@alexandrudima oh, now that I actually look into the menu code, this seems very fishy:

// We make sure to not run actions when the window has no focus, this helps
// for https://github.com/Microsoft/vscode/issues/25907 and specifically for
// https://github.com/Microsoft/vscode/issues/11928
// Still allow to run when the last active window is minimized though for
// https://github.com/Microsoft/vscode/issues/63000
let activeWindow = this.windowsMainService.getFocusedWindow();
if (!activeWindow) {
	const lastActiveWindow = this.windowsMainService.getLastActiveWindow();
	if (lastActiveWindow && lastActiveWindow.isMinimized()) {
		activeWindow = lastActiveWindow;
	}
}

@sbatten it looks like we fail to send the action to the last active window at least? any particular reason?

E.g. when you check the implementation of sendToFocused it will do this:

const focusedWindow = this.getFocusedWindow() || this.getLastActiveWindow();

I wonder if we should change IWindowsService#getFocusedWindow to do that so that we always have a window.

@sbatten
Copy link
Member

sbatten commented Aug 21, 2019

@bpasero based on the issues in the comments, it seems like that would break things elsewhere, no? That whole section has you for git blame.

@bpasero
Copy link
Member

bpasero commented Aug 21, 2019

Ok, I think it is fine to leave the code as it is but then I wonder why starting Code from the command line would not correctly put focus into the window that opens. Or at least, why Electron would not return the window back that is opening.

To clarify: it is the same window that gets opened, not a new one right?

Do people see the window with inactive title color? Does it visually look focused?

@bbugh
Copy link
Author

bbugh commented Aug 21, 2019

Correct, it is the same OS window that gets opened. VS Code is visually (and behaviorally) active, I am able to move the cursor, type, run commands, select text, etc.

However, while testing your inquiry I found a new data point. This issue only happens if the file is already open in an editor. If the file is being opened into a new editor, it works as expected.

Reproduction

  1. Close all editor groups
  2. Cmd+Click open a file with a name/line number (like Gemfile:18)
  3. Notice Cmd+D works.
  4. Return to terminal, Cmd+Click the same file name/line number
  5. Notice Cmd+D does not work.
  6. Open any QuickPick (Cmd+T, Cmd+P, etc.)
  7. Close the QuickPick with Escape or by clicking anywhere else on the Code window.
  8. Notice Cmd+D works again

Related shortcut keys not working

I noticed while doing these reproduction steps, that this issue also affects the Shift+Cmd+L "Select All Occurrences" shortcut.

Additional Testing

I wondered if this issue affected other "what's under the cursor?" commands, and they seem to be working fine.

  • I activated "Developer: Inspect TM Scopes" with a shortcut key, and it showed the correct scope in both scenarios.
  • "Go to Symbol in Workspace" with Cmd+T (which defaults to whatever is under the cursor) and it correctly read what was under the cursor.

Hope this helps.

@bpasero
Copy link
Member

bpasero commented Aug 21, 2019

@bbugh do other keybindings like Cmd+W work in that case and close the editor? Because if yes, then this would be an issue inside the workbench/editor and not outside.

@bbugh
Copy link
Author

bbugh commented Aug 21, 2019

Yes, all of the other editor shortcuts I’ve tried work fine. I usually don’t even notice an issue opening files from the terminal unless I try to select occurrences. The issue seems to be isolated to the “select matches” behavior shortcuts.

@bpasero
Copy link
Member

bpasero commented Aug 21, 2019

@alexandrudima is there anything specific to that command compared to closing editor?

@alexdima
Copy link
Member

No, not that I can think of.

@bpasero
Copy link
Member

bpasero commented Aug 23, 2019

@bbugh can you do an experiment and run "code --verbose" which will print a log statement to the console everytime a command is executed and then check the dev tools console for output? Should look like this:

before

@bbugh
Copy link
Author

bbugh commented Aug 23, 2019

@bpasero Here's a gist with the logs, reproducing the issue. I put a break where I first opened the file (Cmd+D works), then another break when I opened it the second time (when Cmd+D doesn't work). Both times I selected a word and tried to select all instances, but I didn't see any logs related to that.

https://gist.github.com/bbugh/b01c2c591fbd9a6afeb86ea1c2271a1f

@bbugh
Copy link
Author

bbugh commented Aug 23, 2019

@bpasero the image didn't load on my phone earlier and I misunderstood what you meant. I re-ran vscode with code --verbose, then opened the developer console. I reproduced the issue.

When the issue is happening, Cmd+D does not show anything in the developer console. When the issue is not happening, CommandService#executeCommand editor.action.addSelectionToNextFindMatch correctly shows up in the console.

It appears that something is preventing the command from activating altogether.

@bpasero
Copy link
Member

bpasero commented Aug 24, 2019

@bbugh would you be willing to try to reproduce this running VSCode out of sources so that we can debug this further? There are instructions at https://github.com/microsoft/vscode/wiki/How-to-Contribute

@bpasero bpasero added the info-needed Issue requires more information from poster label Aug 24, 2019
@vscodebot vscodebot bot closed this as completed Aug 31, 2019
@vscodebot
Copy link

vscodebot bot commented Aug 31, 2019

This issue has been closed automatically because it needs more information and has not had recent activity. See also our issue reporting guidelines.

Happy Coding!

@bpasero bpasero reopened this Aug 31, 2019
bpasero added a commit that referenced this issue Sep 2, 2019
@bpasero
Copy link
Member

bpasero commented Sep 4, 2019

@bbugh can you do this:

  • download and run this build
  • from the command line with code-insiders --verbose
  • do your keybinding thing until it fails
  • attach the output you see from the console

I added some logging for forwarding commands to the window.

@bbugh
Copy link
Author

bbugh commented Sep 4, 2019

@bpasero thank you for the reminder.

  1. I created a new user account so there would be external/configuration interference.
  2. I downloaded that build and ran with --verbose
  3. I was unable to reproduce the issue using your insider 1.38 1.39 version.
  4. I was able to immediately reproduce the issue with the installed latest public 1.37.

I recorded the reproduction if you'd like to see it, but it seems like 1.38 1.39 fixed the issue?

@bpasero
Copy link
Member

bpasero commented Sep 4, 2019

@bbugh hm it should be version 1.39 though, can you check? Could it be by accident you got updated (actually downgraded)? Maybe disable updates from settings to be safe.

image

@bbugh
Copy link
Author

bbugh commented Sep 4, 2019

Sorry, you're right, it was 1.39.

Version: 1.39.0-insider
Commit: 109add76dad3dd1cbe05215d26cb62be6dbfdc47
Date: 2019-09-04T05:24:38.192Z
Electron: 4.2.10
Chrome: 69.0.3497.128
Node.js: 10.11.0
V8: 6.9.427.31-electron.0
OS: Darwin x64 18.7.0

@bpasero
Copy link
Member

bpasero commented Sep 4, 2019

Wow good to know, looks like my changes fixed it then.

@bpasero bpasero added this to the September 2019 milestone Sep 4, 2019
@bpasero
Copy link
Member

bpasero commented Sep 5, 2019

@bbugh still good with latest insider release from today? Then I would go ahead and close this.

@bpasero bpasero closed this as completed Sep 10, 2019
@bpasero bpasero added bug Issue identified by VS Code Team member as probable bug and removed info-needed Issue requires more information from poster labels Sep 10, 2019
@bpasero
Copy link
Member

bpasero commented Sep 10, 2019

Please report back otherwise.

@bpasero
Copy link
Member

bpasero commented Sep 10, 2019

Actually duplicate of #71553 (comment)

@bpasero bpasero added the *duplicate Issue identified as a duplicate of another issue(s) label Sep 10, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug *duplicate Issue identified as a duplicate of another issue(s) keybindings VS Code keybinding issues macos Issues with VS Code on MAC/OS X
Projects
None yet
Development

No branches or pull requests

5 participants