Comments (12)
@msujew I am chiming in. Apologies in advance if I misunderstood you but I more than often solved these issues with simple helpers.
So this:
streamAllContents(rule).forEach(e => {
if (isRuleCall(e.node) && isParserRule(e.node.rule.ref)) {
accept('error', 'Primitive rules can only call other primitive rules or terminal rules', { node: e.node });
}
if (isAction(e.node)) {
accept('error', 'Primitive rules cannot contain actions.', { node: e.node });
}
if (isAssignment(e.node)) {
accept('error', 'Primitive rules cannot contain assignments.', { node: e.node });
}
});
can be written as:
streamAllContents(rule).forEach(patternMatch( [
['ParserRule', parserRule => ...],
['Action', action => ...],
...
]));
You have to write the patterMatch function of course. But you only have to write it once, it is not that hard to write, and you can use the functional library Ramda to do so if you are proficient in functional programming. I did that a number of times in less time that it takes to find a library that does it for me. The pattern matching pattern occurs fairly often.
Bottom-line is, as a fellow open-source maintainer, I generally support keeping the surface API of a library low, and pusing to userland what can be done there. So I would tend to agree with @danieldietrich here.
from langium.
to just go through some objects
You mean object that e.g. have the same type?
from langium.
Right, for example, if I want to process every Assignment within a rule.
from langium.
I like the idea to iterate only once over the AST and all "registered" methods (or participants) for e.g. Assigment are than called. I would love to see a TS sketch for that
from langium.
I don't quite get it. Can you give an example that you can't realize with streamAllContents
?
from langium.
Not really, since I thought to implement it via streamAllContents
. The visitor would only be an easier way to access everything. But if your opinion is that streamAllContents
is enough, then we can close this issue.
from langium.
Ok of course we can implement additional utility on top of that.
from langium.
I would think twice before adding multiple concepts for the same thing.
This 'visitor' pattern can be described as .filter
followed by a .map
(resp. .forEach
, which is just a map returning nothing/void). In Scala for example, switch / cases
are partial functions (the opposite of total functions), they are defined for certain elements. This is why Scala introduced collect
as shorthand for filter
+ map
.
But maybe we can do it even simpler. If map
would return some type U | void
, then we could consume elements like this:
stream.map(node => {
if (node.isTypeA()) {
consumeTypeA(node);
} else if (node.isTypeB()) {
consumeTypeB(node);
}
})
OR just use forEach
😊.
Introducing a completely new concept is too heavy-weight IMO. Also the visitor pattern is very much OO and not so scalable. You need to provide visit methods for each kind of type, which is very verbose. We would not gain anything with a visitor, it would (internally) iterate the whole tree, though.
What do you think?
from langium.
What I'm doing right know looks something like this:
streamAllContents(rule).forEach(e => {
if (isRuleCall(e.node) && isParserRule(e.node.rule.ref)) {
accept('error', 'Primitive rules can only call other primitive rules or terminal rules', { node: e.node });
}
if (isAction(e.node)) {
accept('error', 'Primitive rules cannot contain actions.', { node: e.node });
}
if (isAssignment(e.node)) {
accept('error', 'Primitive rules cannot contain assignments.', { node: e.node });
}
});
But with a visitor I'd have something like this:
const visitor = new LangiumGrammarVisitor();
visitor.on('ParserRule', parserRule => ...);
visitor.on('Action', action => ...);
visitor.on('Assignment', assignment => ...);
visitor.visit(rule);
The second approach is way cleaner.
Introducing a completely new concept is too heavy-weight IMO. Also the visitor pattern is very much OO and not so scalable. You need to provide visit methods for each kind of type, which is very verbose.
Well, not really, see the example above. With TypeScript/JavaScript we can create much more concise visitors than in Java or other heavy OO languages. This design is inspired by how usual JavaScript listener/callback based frameworks operate instead of the usual OO stuff.
from langium.
const visitor = new LangiumGrammarVisitor();
visitor.on('ParserRule', parserRule => ...);
visitor.on('Action', action => ...);
visitor.on('Assignment', assignment => ...);
visitor.visit(rule);
That is almost what I had in mind when reading this feature request. What I would expect to see from the generated implementation is that register functions (here on
) are generated as onAction
, onAssigment
a.s.o. The idea behind it is that the existing code breaks as soon as you e.g. deletes Action
or rename it to something else
from langium.
@msujew I actually like the first variant more. It is more direct and does not require to understand the additional pattern.
from langium.
Thank you @brucou for your input. I guess you are right, using a pattern matching method in there makes more sense than going through the trouble of building a dedicated visitor. I'll close this issue.
from langium.
Related Issues (20)
- Type Inference and Subtype with Discriminator
- enhance the default keywords autocomplete HOT 3
- Allow to extend the generated textMate grammar HOT 1
- Langium documents don't get removed after the containing directory is renamed HOT 4
- update `vscode-languageserver` and `vscode-languageclient` to `v9`
- Custom rendering of tags in JSDoc comments HOT 1
- `Maximum call stack size exceeded` because of missing assignments HOT 1
- npm run langium:generate does not generate optional field in base class but does in derived class, resulting in TypeScript error
- Duplicated cases in generated `getReferenceType`
- cli command shown in "langium-quickstart.md" is not correct HOT 7
- Add features for the Arithmetic example
- Misleading type compatibility validation error
- Add features for the Statemachine example HOT 3
- Enforce isolation of package exports HOT 2
- Parsing EOF leads to invalid CST ranges
- Inconsistent behavior in the completion provider service
- Add service for text document handling HOT 1
- Type hierarchy provider HOT 1
- Testing facilities generated out of the box HOT 1
- Move generator API to separate export HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from langium.