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 > Move to a new file -- not working (update, nothing in Refactor menu working) #58711

Closed
babakness opened this issue Sep 14, 2018 · 19 comments · Fixed by #59059
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug editor-code-actions Editor inplace actions (Ctrl + .) javascript JavaScript support issues typescript Typescript support issues verified Verification succeeded

Comments

@babakness
Copy link

babakness commented Sep 14, 2018

  • VSCode Version: 1.27.2 (1.27.2)
  • OS Version: MacOS 10.13.5

Also not working

  • VsCode Version: Version 1.28.0-insider

Steps to Reproduce:

1. Highlight a function
2. Select refactor > move to new file

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

Yes -- meaning it also fails to work with --disable-extensions

--

I'm not sure why, the feature was working just fine before. I also tried setting "typescript.tsserver.trace": "verbose"

Tried on multiple projects

I see nothing in the Output pane.

This feature was working prior to update.

-- UPDATE:

Also not working on the refactor menu:

  1. Convert named export to default export
  2. Extract function to module scope
  3. Extract constant to enclosing scope
@babakness babakness changed the title Refactor > Move to new file -- not working Refactor > Move to a new file -- not working (update, nothing in Refactor menu working) Sep 14, 2018
@babakness
Copy link
Author

Update: can confirm the feature works on VS Code Version 1.25.1 (1.25.1), does not work on latest.

@mjbvz mjbvz added typescript Typescript support issues javascript JavaScript support issues editor-code-actions Editor inplace actions (Ctrl + .) bug Issue identified by VS Code Team member as probable bug labels Sep 18, 2018
@mjbvz
Copy link
Collaborator

mjbvz commented Sep 18, 2018

So even after setting typescript.tsserver.trace, you don't see anything in the typescript section of the output panel:

screen shot 2018-09-18 at 2 22 48 pm

@mjbvz mjbvz added the info-needed Issue requires more information from poster label Sep 18, 2018
@babakness
Copy link
Author

Ah, yeah I see it now. I get this output just my highlighting text (not actually selecting "Move to new file"). I get no output when actually selecting Move To New File. Nothing happens. Works on VS Code 1.25 just fine (my current work around is to use this older build)

[Trace  - 3:11:38 PM] Sending request: quickinfo (47). Response expected: yes. Current queue length: 0
Arguments: {
    "file": "...",
    "line": 10,
    "offset": 18
}
[Trace  - 3:11:38 PM] Response received: quickinfo (47). Request took 1 ms. Success: false . Message: No content available.
[Trace  - 3:11:38 PM] Sending request: quickinfo (48). Response expected: yes. Current queue length: 0
Arguments: {
    "file": "...",
    "line": 10,
    "offset": 17
}
[Trace  - 3:11:38 PM] Response received: quickinfo (48). Request took 1 ms. Success: false . Message: No content available.
[Trace  - 3:11:43 PM] Sending request: getApplicableRefactors (49). Response expected: yes. Current queue length: 0
Arguments: {
    "file": "...",
    "startLine": 10,
    "startOffset": 1,
    "endLine": 11,
    "endOffset": 1
}
[Trace  - 3:11:43 PM] Response received: getApplicableRefactors (49). Request took 8 ms. Success: true 
Result: [
    {
        "name": "Convert export",
        "description": "Convert named export to default export",
        "actions": [
            {
                "name": "Convert named export to default export",
                "description": "Convert named export to default export"
            }
        ]
    },
    {
        "name": "Extract Symbol",
        "description": "Extract function",
        "actions": [
            {
                "description": "Extract to function in module scope",
                "name": "function_scope_0"
            }
        ]
    },
    {
        "name": "Extract Symbol",
        "description": "Extract constant",
        "actions": [
            {
                "description": "Extract to constant in enclosing scope",
                "name": "constant_scope_0"
            }
        ]
    },
    {
        "name": "Move to a new file",
        "description": "Move to a new file",
        "actions": [
            {
                "name": "Move to a new file",
                "description": "Move to a new file"
            }
        ]
    }
]

@mjbvz mjbvz added this to the September 2018 milestone Sep 18, 2018
@mjbvz
Copy link
Collaborator

mjbvz commented Sep 18, 2018

You can switch the typescript version independently of VS Code by following these instructions. In the latest VS Code insiders build, can you please try:

  • Using [email protected]. This was the version shipped with VS Code 1.25
  • Using typescript@next. This has the latest fixes

Do either of those fix the problem?

Let me know if you have any questions about switching the typescript version in your workspace.

@mjbvz
Copy link
Collaborator

mjbvz commented Sep 19, 2018

Also, does this happen for every refactoring you try to trigger? Does it happen in a new project if you just try to extract a simple expression like 1 + 2 to a constant or function?

@babakness
Copy link
Author

Hey, actually I'm using Typescript 3.0.3 on VS Code 1.25 by choosing the "Workspace" Typescript--and refactoring works as expected.

1 + 2 refactor does not work in VSC 1.27, does work in VSC 1.25

@babakness
Copy link
Author

babakness commented Sep 19, 2018

To follow up, every option on the refactoring menu is not working in the latest VS Code, including the "Insiders" edition with no plugins or extensions running.

All options work as expected on VS Code 1.25, I've not tried VS Code 1.26

@mjbvz
Copy link
Collaborator

mjbvz commented Sep 20, 2018

Thanks. Unfortunately I'm unable to reproduce this on any machine so I need a bit more information from you:

  1. What are the exact steps to take to show the code action menu and the select the desired refactoring? Are you using the mouse? The keyboard? Any specific keyboard configuration or international layout?

  2. In a TypeScript file, if you write:

class Foo {}
class Bar extends Foo {
    constructor() {}
}

Can you successfully trigger the quick fix that adds the missing super call in the constructor?

  1. Try collecting VS Code's internal logs for executing the quick fix:

    1. In VS Code, run the command Developer: Set log level...
    2. Select trace. Restart VS Code and reproduce the problem
    3. Back in the output panel, share the output from the Log (Extension Host) and Log (Window) sections

@babakness
Copy link
Author

Hey, so I just noticed something. If I click on the lightbulb icon when I select code, it works! But it doesn't work if I select the code, right click on the selected code, choose "Refactor..." then choose "Move To New File"

Log output

[2018-09-19 18:46:36.855] [renderer1] [info] no standard startup: not the explorer viewlet
[2018-09-19 18:46:53.619] [renderer1] [info] no standard startup: panel is active

Nothing else!

@babakness
Copy link
Author

Oh here is potentially useful log, last one was from Log (window) this one is from Log (Extension Host)

[2018-09-19 18:51:37.206] [exthost1] [warning] vscode.typescript-language-features -Code actions of kind 'refactor 'requested but returned code action is of kind 'quickfix'. Code action will be dropped. Please check 'CodeActionContext.only' to only return requested code actions.

@mjbvz
Copy link
Collaborator

mjbvz commented Sep 20, 2018

Thanks @babakness. I can repo now using the refactoring menu

@sbatten and @bpasero I believe I have tracked this down to the refactor context menu seemingly not firing its actions when the user select an item. This may because the refactor context menu is triggered from within another context menu. I'll keep debugging tomorrow but let me know if you have any ideas on what may be going wrong here

@bpasero
Copy link
Member

bpasero commented Sep 20, 2018

@mjbvz does it reproduce with the custom menu but not the native menu?

@mjbvz
Copy link
Collaborator

mjbvz commented Sep 20, 2018

This is on MacOS, so native context menus

(Haven't been able to repo on windows at all yet)

@bpasero
Copy link
Member

bpasero commented Sep 20, 2018

@mjbvz that would be very weird. I am not even able to bring up anything when I click on "Refactoring...", is this broken? E.g. I selected some code and triggered it and was at least expecting something like "Extract method".

@bpasero
Copy link
Member

bpasero commented Sep 20, 2018

Ok I can reproduce, steps:

  • click "Source actions..."
  • click "Organize Imports"

=> nothing happens

Works fine when invoked from the command palette.

Reproduces in stable for me.

@bpasero
Copy link
Member

bpasero commented Sep 20, 2018

@mjbvz I verified that the "Organize Imports" action is triggered, so it does not seem to be an issue with the menu.

@mjbvz
Copy link
Collaborator

mjbvz commented Sep 20, 2018

This is a bug with how we handle context menus. I added these log points in vscode/src/vs/base/parts/contextmenu/electron-browser/contextmenu.ts

screen shot 2018-09-20 at 9 35 12 am

which shows the following sequence of events.

screen shot 2018-09-20 at 9 30 30 am

To summarize:

  1. Open the editor context menu
  2. Select the refactoring option
  3. Which opens another context menu
  4. At this point we receive a generic context menu closed event.
  5. This event removes both the original editor context menu's click handler and the click handler for the refactor context menu we just created

This explains why adding a slight delay when creating the refactor context menu fixes the problem

@bpasero
Copy link
Member

bpasero commented Sep 20, 2018

This event removes both the original editor context menu's click handler and the click handler for the refactor context menu we just created

Where?

@mjbvz
Copy link
Collaborator

mjbvz commented Sep 20, 2018

The editor and refactor menus both register a close event handler. When the editor context menu is closed, both of the menus receive the same close event so we end up removing the refactor menu’s click handler too even though is only just started showing

@mjbvz mjbvz removed the info-needed Issue requires more information from poster label Sep 20, 2018
mjbvz added a commit to mjbvz/vscode that referenced this issue Sep 20, 2018
Fixes microsoft#58711

When a context menu is triggered from inside another context menu, we have a race condition related to ipc. This is cause by the `close` event for context menus being global. In the retrigger case, `close` ends up being fired after the second context menu is created. This ends up removing the click handler for the new context menu

Fix is to add a menu id to the close event.
bpasero pushed a commit that referenced this issue Sep 21, 2018
Fixes #58711

When a context menu is triggered from inside another context menu, we have a race condition related to ipc. This is cause by the `close` event for context menus being global. In the retrigger case, `close` ends up being fired after the second context menu is created. This ends up removing the click handler for the new context menu

Fix is to add a menu id to the close event.
@alexr00 alexr00 added the verified Verification succeeded label Sep 28, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 5, 2018
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 editor-code-actions Editor inplace actions (Ctrl + .) javascript JavaScript support issues typescript Typescript support issues verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants