Skip to content

Support for UserQuestionException during reloads#7904

Merged
sdedic merged 1 commit intoapache:masterfrom
sdedic:lsp/reload-branding
Jan 15, 2025
Merged

Support for UserQuestionException during reloads#7904
sdedic merged 1 commit intoapache:masterfrom
sdedic:lsp/reload-branding

Conversation

@sdedic
Copy link
Copy Markdown
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
Copy Markdown
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.

private AskEditorQuestions() {
}

public static Boolean askUserQuestion(UserQuestionException uqe) {
Copy link
Copy Markdown
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
Copy Markdown
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
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