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

Introduce delegate command for java file paste #2981

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

hopehadfield
Copy link
Contributor

Adds a delegate command that identifies whether or not some given clipboard text is a piece of java text and returns the correct path to the file to be created given relevant package declarations and/or type name.

Addresses redhat-developer/vscode-java#3323.

Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks pretty good. I'll have a closer look at the package logic later.

Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks good. Just some things to clean up.

@robstryker
Copy link
Contributor

It looks like you addressed everything @rgrunber mentioned and what I mentioned as well.

Just a small comment: sometimes when getting reviews, its nice to leave the fixes as a separate commit so reviewers can go verify the specific changes made in response to the review. We can always squash later or even squash as part of the merge button here in github.

But if you squash it first and force-push and essentially delete the previous version, it becomes harder to see what you changed upon review.

But again, seems like you fixed everything that was mentioned. +1 from me (although I admit to not using the ASTParser much just yet.)

@fbricon
Copy link
Contributor

fbricon commented Dec 11, 2023

@robstryker I use the compare commit link for that purpose
Screenshot 2023-12-11 at 19 39 43

@rgrunber
Copy link
Contributor

rgrunber commented Dec 11, 2023

Yeah, when I discovered the "force-pushed" was a link (eg. https://github.com/eclipse-jdtls/eclipse.jdt.ls/compare/0ab10ab5662c390309dc21882f1f92d02a769b52..c708293501fa5b8816675137516453450a93b833 ) it became easier to review though I agree that individual commits are more explicit.

@rgrunber
Copy link
Contributor

I get the following error on Windows :

!ENTRY org.eclipse.jdt.ls.core 4 0 2023-12-13 00:50:08.563
!MESSAGE Error in calling delegate command handler
!STACK 0
java.lang.IllegalArgumentException: character to be escaped is missing
	at java.base/java.util.regex.Matcher.appendExpandedReplacement(Unknown Source)
	at java.base/java.util.regex.Matcher.appendReplacement(Unknown Source)
	at java.base/java.util.regex.Matcher.replaceAll(Unknown Source)
	at java.base/java.lang.String.replaceAll(Unknown Source)
	at org.eclipse.jdt.ls.core.internal.handlers.PasteEventHandler.searchForMatchingPackage(PasteEventHandler.java:413)
	at org.eclipse.jdt.ls.core.internal.handlers.PasteEventHandler.handleFilePasteEvent(PasteEventHandler.java:363)
	at org.eclipse.jdt.ls.core.internal.JDTDelegateCommandHandler.executeCommand(JDTDelegateCommandHandler.java:201)
	at org.eclipse.jdt.ls.core.internal.handlers.WorkspaceExecuteCommandHandler$1.run(WorkspaceExecuteCommandHandler.java:230)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:47)
	at org.eclipse.jdt.ls.core.internal.handlers.WorkspaceExecuteCommandHandler.executeCommand(WorkspaceExecuteCommandHandler.java:220)
	at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.lambda$4(JDTLanguageServer.java:616)
	at org.eclipse.jdt.ls.core.internal.BaseJDTLanguageServer.lambda$0(BaseJDTLanguageServer.java:87)
	at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(Unknown Source)
	at java.base/java.util.concurrent.CompletableFuture$Completion.run(Unknown Source)
	at java.base/java.lang.Thread.run(Unknown Source)

Signed-off-by: Hope Hadfield <hhadfiel@redhat.com>
@rgrunber
Copy link
Contributor

rgrunber commented Dec 13, 2023

!ENTRY org.eclipse.jdt.ls.core 4 0 2023-12-13 16:51:34.982
!MESSAGE Error in calling delegate command handler
!STACK 0
java.nio.file.InvalidPathException: Illegal char <:> at index 2: /c:/Users/user/projects/foo\org\foo
	at java.base/sun.nio.fs.WindowsPathParser.normalize(Unknown Source)
	at java.base/sun.nio.fs.WindowsPathParser.parse(Unknown Source)
	at java.base/sun.nio.fs.WindowsPathParser.parse(Unknown Source)
	at java.base/sun.nio.fs.WindowsPath.parse(Unknown Source)
	at java.base/sun.nio.fs.WindowsFileSystem.getPath(Unknown Source)
	at java.base/java.nio.file.Path.of(Unknown Source)
	at java.base/java.nio.file.Paths.get(Unknown Source)
	at org.eclipse.jdt.ls.core.internal.handlers.PasteEventHandler.searchForMatchingPackage(PasteEventHandler.java:415)
	at org.eclipse.jdt.ls.core.internal.handlers.PasteEventHandler.handleFilePasteEvent(PasteEventHandler.java:364)
	at org.eclipse.jdt.ls.core.internal.JDTDelegateCommandHandler.executeCommand(JDTDelegateCommandHandler.java:201)
	at org.eclipse.jdt.ls.core.internal.handlers.WorkspaceExecuteCommandHandler$1.run(WorkspaceExecuteCommandHandler.java:230)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:47)
	at org.eclipse.jdt.ls.core.internal.handlers.WorkspaceExecuteCommandHandler.executeCommand(WorkspaceExecuteCommandHandler.java:220)
	at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.lambda$4(JDTLanguageServer.java:616)
	at org.eclipse.jdt.ls.core.internal.BaseJDTLanguageServer.lambda$0(BaseJDTLanguageServer.java:87)
	at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(Unknown Source)
	at java.base/java.util.concurrent.CompletableFuture$Completion.run(Unknown Source)
	at java.base/java.lang.Thread.run(Unknown Source)

Update: I don't think this is coming from the language server. The code seems to be correct, so I think it's an incorrect value passed by the client-side code.

@rgrunber rgrunber merged commit 3a74fa2 into eclipse-jdtls:master Dec 14, 2023
7 checks passed
@rgrunber rgrunber added this to the End December 2023 milestone Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants