-
Notifications
You must be signed in to change notification settings - Fork 15
RFC for table interpretation #128
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
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.
I see I was removed but I like the concept + the RFC presentation is great and clear, amazing.
The only thing I am unsure about is (as you already point out) the double semantics of "col1" typed text
with and without header is weird but I also have no really better idea right now. So I'd go for this and we can revisit it later.
block MyTableInterpreter oftype TableInterpreter { | ||
header: false; | ||
columns: [ | ||
"col1" typed text, | ||
"col2" typed integer, | ||
"col3" typed boolean, | ||
"col4" typed decimal, | ||
] | ||
} | ||
``` | ||
|
||
The attribute `header: false` indicates that the sheet has no topmost header row, so the column names are taken from the | ||
provided names in the `columns` attribute. There, each entry describes a column by defining its name and its type. Note | ||
that the order of columns matters, as the first entry maps to the first column `A`, the second to `B` and so on. |
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.
Nice! My only concern is the semantic "the order matters" in an array syntax []
. That might be counter-intuitive for programmers, we could offer some clarification elsewhere, though, e.g. in the VSCode plugin adding a gray "A" in front or so.
@rhazn @georg-schwarz thanks for the feedback! Please let me know whether we need a further discussion or you consider this accepted. Additionally, I'd like to share my thoughts regarding semantic ordering differences:
To resolve the difference, we could
Then the ordering in |
Imho having ordering matter in the array itself is not an issue. I think it would be nice to separate naming columns from using the names. This way the typing semantics would always be the same (name and type). Then you could get the names by either Either way, if you go with your choice here or my suggestion, I'd consider both fine. |
I'm fine with either way, also with the original if we document it accordingly. |
I really like the proposal from @georg-schwarz with having IDE hints that display the corresponding column. I researched a bit and found the so called inlay hints. See a VS Code demo video and the corresponding LSP specification. Not sure though how easily that can be implemented using Langium. Edit: Langium has no native support currently but it is planned to provide an interface for them, see eclipse-langium/langium#595. |
I'd caution against that. It is cool but we are fundamentally developing a language, not an IDE. So if the language is unclear/hard to understand without VSCode I think we are making a mistake. I am aware that implementing it with the LSP means the feature is not bound to VSCode but it is bound to the user installing a specific tool to deal with .jv files. |
Then I propose to go with the table building concept as stated here in the RFC, without further changes. Of course we can revisit the approach and improve it in the future. |
👍 |
Part of #85