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

Let the user choose main class, when none is open #5536

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

jlahoda
Copy link
Contributor

@jlahoda jlahoda commented Feb 19, 2023

In the VS Code, when attempting to start a program using the Run pane, without any file opened, the Run fails, as no main class is found.

This patch proposes to show a dialog so that the user can choose a main class. In case only one main class is found, it is selected automatically.

The intent behind the getServerLookup() -> getServerServicesLookup() change is to ensure the content of the default lookup is present in context.getLspSession().getLookup() only once. It is added there once in the session lookup, and is also part of the server lookup. So, the patch is instead using a subset of the server lookup that does not include the default lookup.


^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.

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.

@mbien mbien added the LSP [ci] enable Language Server Protocol tests label Feb 20, 2023
@mbien
Copy link
Member

mbien commented Feb 20, 2023

@dbalek @sdedic I added LSP label and going to restart tests so that LSP tests are run. Labels are also important for getting this PR into the release notes. Please check those things before approving to make it less likely that the master is red (which just happened a week ago, so this isn't theoretical - although I have no doubt that jan tested this PR properly).

see wiki for more details

@apache apache locked and limited conversation to collaborators Feb 20, 2023
@apache apache unlocked this conversation Feb 20, 2023
@mbien mbien added this to the NB18 milestone Feb 20, 2023
@apache apache locked and limited conversation to collaborators Feb 20, 2023
@apache apache unlocked this conversation Feb 20, 2023
@matthiasblaesing
Copy link
Contributor

@jlahoda this is reviewed - any reason not to merge?

@jlahoda
Copy link
Contributor Author

jlahoda commented Mar 23, 2023

I realized yesterday I'd like one more tweak here (more appropriate place to set the lookup), I'm working on that.

@mbien
Copy link
Member

mbien commented Apr 1, 2023

would recommend to squash before merge.

btw @jlahoda would you be ok with #5617, since we will probably turn off the github squash feature, which would require to squash locally as last step before pressing merge.

@neilcsmith-net neilcsmith-net modified the milestones: NB18, NB19 Apr 18, 2023
@jlahoda jlahoda merged commit 5c5cce4 into apache:master Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LSP [ci] enable Language Server Protocol tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants