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

Supporting source file launcher in VS Code extension #6262

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

lahodaj
Copy link
Contributor

@lahodaj lahodaj commented Jul 27, 2023

(Single) source file launcher support has multiple problems inside VS Code - especially settings that are inside NetBeans done through file attributes are not accessible. This patch attempts to use the Run Configuration tab/settings to provide options for launching the single file. The same options are also parsed for use in the editor.


^Add meaningful description above

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

  • are all your own work, and you have the right to contribute them.
  • are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

@lahodaj lahodaj added the Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) label Jul 27, 2023
@lahodaj lahodaj requested a review from dbalek July 27, 2023 10:41
@lahodaj lahodaj added the VSCode Extension [ci] enable VSCode Extension tests label Jul 27, 2023
@dbalek dbalek requested a review from sdedic July 27, 2023 10:58
@dbalek dbalek added the LSP [ci] enable Language Server Protocol tests label Jul 27, 2023
if (!(argumentsObject instanceof String)) {
return null;
}
return Arrays.asList(((String) argumentsObject).trim().split(" ")); //NOI18N
Copy link
Member

Choose a reason for hiding this comment

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

Maybe handle possible escaping (`BaseUtilities.parseParameters ?)

compileClassPathImplementation.setDelegates(spec2CP(classpath));
compileModulePathImplementation.setDelegates(spec2CP(modulepath));

System.err.println("compileClassPath: " + compileClassPath);
Copy link
Member

Choose a reason for hiding this comment

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

Debugging leftovers

case "--module-path": case "-p":
modulepath = parameter;
break;
case "--source":
Copy link
Member

Choose a reason for hiding this comment

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

also -source with 1 dash

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, but I don't think it should be here - these are source launcher options (as opposed to javac options), and source launcher does not support -source.

String sourceLevel = SourceLevelResultImpl.DEFAULT_SL;

for (int i = 0; i < newOptions.size(); i++) {
if (i + 1 >= newOptions.size())
Copy link
Member

Choose a reason for hiding this comment

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

Maybe limit the loop to just newOptions.size() -1 ?

}

if (compilerArgs.contains(SOURCE_FLAG)) {
compilerArgs = m.replaceAll("--enable-preview " + SOURCE_FLAG + " " + realNewSourceLevel);
Copy link
Member

Choose a reason for hiding this comment

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

Use the constant ENABLE_PREVIEW_FLAG ?

@lahodaj
Copy link
Contributor Author

lahodaj commented Jul 27, 2023

@sdedic thanks a lot for the comments!. I've mostly (except the -source) reflected them under bc9417f.

@arvindaprameya
Copy link

This looks good, thank you for making this change! Appoved

@lahodaj lahodaj force-pushed the source-launcher-in-lsp branch from bc9417f to 42caa8a Compare July 31, 2023 11:07
Copy link
Contributor

@dbalek dbalek left a comment

Choose a reason for hiding this comment

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

Looks fine. Thanks.

@lahodaj lahodaj merged commit f06522f into apache:master Jul 31, 2023
@mbien mbien added this to the NB20 milestone Nov 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants