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

Fill in package for newly created file #1222

Closed
jrieken opened this issue Jan 3, 2020 · 12 comments
Closed

Fill in package for newly created file #1222

jrieken opened this issue Jan 3, 2020 · 12 comments
Assignees

Comments

@jrieken
Copy link

jrieken commented Jan 3, 2020

When creating a new file, e.g Foo.java inside the com.mycomp.app-folder then the extension should use the new vscode.workspace.on[Will|Did]CreateFile-api to fill in some default content, esp the package-declaration package com.mycomp.app etc.

Environment
  • Java extension pack 0.8.1
Steps To Reproduce
  1. Use the vscode file explorer to create a new file
Current Result

File is empty

Expected Result

File should be pre-populated

@akaroml
Copy link
Contributor

akaroml commented Jan 16, 2020

@testforstephen will adopt this.

@testforstephen
Copy link
Collaborator

From the API documentation, the file events could be from file explorer, or workspace.applyEdit-api.

If the new file is created from file explorer, it makes sense to pre-populate some contents based on the context.

But if the new file is from workspace.applyEdit operation, and populating some contents by the client watcher is probably not a good choice. Generally the applyEdit operation is from quick fix or refactoring, where the workspace edit would contain both file changes and text changes on the same file. And the client just watched that a new file is created, and then adding contents immediately will cause duplicated contents added to the target file.

For example, the Java extension provides a quick fix to create the missing type, where the workspace edit will contain an operation to new a file and a text edit to populate the class contents. The client doesn't need do anything for this file create event.
image

There are similar issues on the rename event. When an user is renaming a class name from the symbol, the Java extension will return a workspace edit to rename its Java file too. It's unnecessary for the client to watch such events to do something.

// @jrieken
My question is whether VS Code API allows me to only watch the file events from file explorer?

@jrieken
Copy link
Author

jrieken commented Mar 24, 2020

No, you cannot know what triggered the event. Still, when using onDid-events then the first round of edits have already happened. E.g a refactoring has already finished and your listener can check the newly created files before sending off another edit, e.g only add the package-statement when the file is empty

@testforstephen
Copy link
Collaborator

this is the ideal situation, but the actual sequence won't support that.

See the sequence i observed:
-> Apply quick fix to add the missing type
-> onDidCreateFiles event
-> didOpen
-> didChange

So at the onDidCreateFiles event, actually the language server didn't receive the previous edit change yet, so cannot tell the file is empty.

@jrieken
Copy link
Author

jrieken commented Mar 24, 2020

See the sequence i observed:

You can "change" the sequence by opening the document while handling the event. The onDid-event doesn't wait for you and it can use these files just as hint as way of "this file has been created, keep this in mind and fill in default when I know more and when it makes sense"

@testforstephen
Copy link
Collaborator

ok, thanks. I managed to exclude non-empty new files via workspace.openTextDocument(uri) api during workspace.onDidCreateFiles handler.

Now regarding to the rename events, it's a little complicated.

  • If it's file name rename operation, then it's workable to update the main class name in the workspace.onDidRenameFiles listener.
  • But if it's a file move operation (e.g. move class from package a to package b), the references (e.g. imports) update calculation need be done before the actual move happens. We may have to use workspace.onWillRenameFiles listener for the file move case. For this scenario, it seems hard to distinguish the event trigger between the file explorer and applyEdit. Are the text changes in the first round of applyEdit visible to the onWillRenameFiles event? // @jrieken

@jrieken
Copy link
Author

jrieken commented Mar 25, 2020

For us, rename and move are the same. So, I don't know how you would differentiate between them?

Are the text changes in the first round of applyEdit visible to the onWillRenameFiles event

Not sure what you mean? Are you concerned that you could have caused the move and that there is nothing that needs to be done? Is there a "move to new folder" refactoring or how does this happen?

@testforstephen
Copy link
Collaborator

I understand rename in VS Code is generic, it's just because our Java implementation uses different logic to handle them. Currently i will check whether the oldUri and newUri is in the same directory to tell it's a rename or move.

yes, the Java extension provides a "move" refactoring in the editor context. And when the menu is clicked, it will prompt a quick pick to tell user to select the destination, after that the language server calculates a workspace edit and apply the move operation. What i want is to exclude such kind of rename event in the client rename listener, because the move refactoring already contains both file rename and import update.

@jrieken
Copy link
Author

jrieken commented Mar 26, 2020

Understood - we could add a reason or source property that tells you what caused the workspace edit, in the spirit of TextDocumentSaveReason or TextEditorSelectionChangeKind. That could tell you if an edit was caused by a user gesture or not.

@jrieken
Copy link
Author

jrieken commented Mar 26, 2020

I have created microsoft/vscode#93470 for this

@testforstephen
Copy link
Collaborator

Cool. that's exactly what i want.

@fbricon fbricon added this to the End March 2020 milestone Mar 31, 2020
@fbricon
Copy link
Collaborator

fbricon commented Mar 31, 2020

Fixed:
newfile

When the project uses JDK 14, the proposals look different to accomodate for the different record content:

image

@fbricon fbricon closed this as completed Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants