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

Support for UserQuestionException during reloads #7904

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Oct 23, 2024

This PR allows to step in even during reload, by throwing BadLocationException with a wrapped UserQuestionException - will result in the question dialog for the user.

For LSP/headless operation, a branding API is introduced that allows to skip question for *specified subclass of UserQuestionException - either always permit or always deny. Applications/distributions can control that using branding.

@sdedic sdedic added API Change [ci] enable extra API related tests Platform [ci] enable platform tests (platform/*) labels Oct 23, 2024
@sdedic sdedic added this to the NB25 milestone Oct 23, 2024
@sdedic sdedic requested review from dbalek and lahodaj October 23, 2024 18:04
@sdedic sdedic self-assigned this Oct 23, 2024
Copy link
Contributor

@lahodaj lahodaj 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 OK to me, with a nit suggestion inline.


public final class AskEditorQuestions {
private AskEditorQuestions() {
}

public static Boolean askUserQuestion(UserQuestionException uqe) {
Copy link
Contributor

Choose a reason for hiding this comment

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

A nit: it might be nicer to use a 3-constant enum instead of a Boolean. The enum could say YES, NO and ASK_USER, making the result more obvious.

@sdedic sdedic force-pushed the lsp/reload-branding branch from cabce67 to 91b30af Compare January 15, 2025 09:44
@sdedic
Copy link
Member Author

sdedic commented Jan 15, 2025

Review comment applied, rebased on latest master + squashed.

@sdedic sdedic merged commit daa8132 into apache:master Jan 15, 2025
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Change [ci] enable extra API related tests Platform [ci] enable platform tests (platform/*)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants