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

feat: expose MicroProfile server settings #1117

Merged
merged 1 commit into from
Aug 28, 2023

Conversation

fbricon
Copy link
Contributor

@fbricon fbricon commented Aug 22, 2023

Part of #794
Screenshot 2023-08-22 at 14 53 36

Signed-off-by: Fred Bricon [email protected]

@angelozerr
Copy link
Contributor

It is out of thescope of this PR, but I wonder if we should remove LSP ?

image

@angelozerr
Copy link
Contributor

angelozerr commented Aug 23, 2023

It works like a charm.

IMHO I think we should improve description of each error with example like HTML does:

image

* Problem severity levels used by LSP4MP
*/
//TODO move to lsp4ij?
public enum ProblemSeverity {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not using DiagnosticSeverity and null for none?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DiagnosticSeverity has other values like hint and info, which could be mapped to IJ's severities, but since we don't use them, eventually I kept ProblemSeverity, as it makes things simpler, less risks of confusion.

@fbricon
Copy link
Contributor Author

fbricon commented Aug 23, 2023

I also need to fix:

com.redhat.devtools.intellij.quarkus.completion.MavenApplicationPropertiesCompletionTest

  Test testBooleanCompletion FAILED (43.3s)

  junit.framework.AssertionFailedError: expected:<2> but was:<0>
      at com.redhat.devtools.intellij.quarkus.completion.MavenApplicationPropertiesCompletionTest.testBooleanCompletion(MavenApplicationPropertiesCompletionTest.java:41)

@fbricon fbricon force-pushed the lsp4mp-settings branch 2 times, most recently from ad39913 to cb71d46 Compare August 23, 2023 09:03
@fbricon
Copy link
Contributor Author

fbricon commented Aug 23, 2023

It is out of thescope of this PR, but I wonder if we should remove LSP ?

Done, it provided no value.

@fbricon fbricon force-pushed the lsp4mp-settings branch 2 times, most recently from ac0a695 to 19f0682 Compare August 23, 2023 10:51
@fbricon
Copy link
Contributor Author

fbricon commented Aug 23, 2023

It works like a charm.

IMHO I think we should improve description of each error with example like HTML does:

Screenshot 2023-08-23 at 14 42 09

Looks worse in dark mode though

Screenshot 2023-08-23 at 14 43 58

WDYT?

@fbricon fbricon force-pushed the lsp4mp-settings branch 4 times, most recently from 173b94a to 8ab24c2 Compare August 23, 2023 15:17
@fbricon fbricon marked this pull request as ready for review August 23, 2023 15:19
@fbricon
Copy link
Contributor Author

fbricon commented Aug 23, 2023

I removed the unassigned inspection, as LSP4MP ignores the unassigned severity (you can't set it neither in vscode)

I'm pondering whether to remove the duplicates inspection as well, since IJ sets the severity to error on its own, even though Properties Duplicates inspection is disabled (probably a bug in IJ)

@angelozerr
Copy link
Contributor

angelozerr commented Aug 25, 2023

WDYT?

It is great! The dark mode should be improved but if you wish we can do that in an another PR.

I think that we should use editor background (it means that it html should be dynamic, why not use Qute to load your HTML file and replace bagkground color with the IJ background color?)

We can do that in a sperate PR if you wish.

@fbricon
Copy link
Contributor Author

fbricon commented Aug 25, 2023

This simpler approach works in both cases, I think:
Screenshot 2023-08-25 at 15 53 05
Screenshot 2023-08-25 at 15 53 34

@angelozerr
Copy link
Contributor

Indeed it is better, the yellow div border should use perhaps a gray color, because in dark mode I find it is a little flashy.

@angelozerr
Copy link
Contributor

@fbricon do you think it is possible and easy to do to add a link the the MicroProfile / Properties page to the Inspections MicroProfile validation settings?

@fbricon
Copy link
Contributor Author

fbricon commented Aug 25, 2023

@fbricon do you think it is possible and easy to do to add a link the the MicroProfile / Properties page to the Inspections MicroProfile validation settings?

I actually thought about it then forgot to do it ;-)

@sonarcloud
Copy link

sonarcloud bot commented Aug 25, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 18 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
11.2% 11.2% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@fbricon
Copy link
Contributor Author

fbricon commented Aug 25, 2023

Darker yellow:
Screenshot 2023-08-25 at 16 53 35
Screenshot 2023-08-25 at 16 53 55

@angelozerr angelozerr merged commit d379c70 into redhat-developer:main Aug 28, 2023
11 of 12 checks passed
@angelozerr
Copy link
Contributor

Thanks so much @fbricon !

@angelozerr angelozerr added the quarkus Quarkus support label Aug 28, 2023
@angelozerr angelozerr added this to the 1.26.0 milestone Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quarkus Quarkus support
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants