-
Notifications
You must be signed in to change notification settings - Fork 822
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
Adding ability to edit database connection properties in VSCode extension #6079
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a pity that the html4j
API was not used.
58e1a9a
to
b0585d7
Compare
bea676c
to
b0585d7
Compare
2b80506
to
d0a0a62
Compare
"unknown": unknown | ||
}; | ||
export type Property<T extends keyof PropTypeMap = keyof PropTypeMap> = T extends T ? { | ||
propPref: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why all the prop
prefixes ? Inconsistent naming: dispName
vs. shortName
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is taken from return type of NBLS command...
...ava.lsp.server/src/org/netbeans/modules/java/lsp/server/explorer/NodePropertiesProvider.java
Show resolved
Hide resolved
...ava.lsp.server/src/org/netbeans/modules/java/lsp/server/explorer/NodePropertiesProvider.java
Show resolved
Hide resolved
...ava.lsp.server/src/org/netbeans/modules/java/lsp/server/explorer/NodePropertiesProvider.java
Show resolved
Hide resolved
if (propJson instanceof JsonNull) { | ||
return CompletableFuture.completedFuture(null); | ||
} | ||
List m = gson.fromJson((JsonElement) propJson, List.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm ... could be more friendly if the property sets was a map (keyed by set name) instead of array, but the propset name is present in each array's item, so it's probably OK.
Maybe if
List<Map<String, Object>> m = ...
the setAllProperties
and others can use strongly typed Map instead of raw type(s) ?
prop.setValue(val); | ||
} | ||
} catch (IllegalAccessException ex) { | ||
Exceptions.printStackTrace(ex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe there could be a way how to report a failed set to the client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least the unnecessary whitespace reformat should be fixed before merge.
df183a2
to
b50d9b3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, let's fix the 'exceptional' return values from nbls later - I've filed #6113 for that.
ad3bce5
to
b6b06dc
Compare
b6b06dc
to
d1f4a1a
Compare
please don't merge PRs without squashing them properly. Not sure what happened here, e.g the merge commit is missing which makes it difficult to track commits back to their PR. |
Adding ability to open property sheet of database connection in VSCode extension.