-
Notifications
You must be signed in to change notification settings - Fork 29
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
#180: Add additional default types and improve arguments handling #105
Conversation
- Use some known arguments in the workflow example Fixes eclipse-glsp/glsp#180 Signed-off-by: Martin Fleck <mfleck@eclipsesource.com>
plugins/org.eclipse.glsp.graph/src/org/eclipse/glsp/graph/builder/GArgumentableBuilder.java
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,83 @@ | |||
/******************************************************************************** | |||
* Copyright (c) 2020 EclipseSource and others. |
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.
* Copyright (c) 2020 EclipseSource and others. | |
* Copyright (c) 2021 EclipseSource and others. |
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.
Good catch!
plugins/org.eclipse.glsp.graph/src/org/eclipse/glsp/graph/builder/GArgumentableBuilder.java
Show resolved
Hide resolved
plugins/org.eclipse.glsp.graph/src/org/eclipse/glsp/graph/builder/GArgumentableBuilder.java
Outdated
Show resolved
Hide resolved
plugins/org.eclipse.glsp.graph/src/org/eclipse/glsp/graph/builder/GArgumentableBuilder.java
Outdated
Show resolved
Hide resolved
|
||
import org.eclipse.emf.common.util.EMap; | ||
|
||
public final class GArguments extends LinkedHashMap<String, Object> { |
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.
In contrast to the GLayoutOptions
the arguments map can hold arbitary key-value pairs so explicit subclassing and limiting the values you can put into the map doesn't make sense Imo.
I think this rather should be a GArgumentsUtil
instead of Map subclass (similar to the util class for ClientOptions
)
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.
I'm not sure I fully agree since the GArguments
map can also hold arbitrary key-value pairs but It's better to be consistent ;-) I changed the structure to a utility class similar to ClientOptions
.
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.
Thanks. Looks good to me 👍
Fixes eclipse-glsp/glsp#180
Signed-off-by: Martin Fleck mfleck@eclipsesource.com