Add a Tree #108
Add a Tree #108
Conversation
| @@ -0,0 +1,414 @@ | |||
|
|
|||
| import { spawn } from 'child_process'; | |||
There was a problem hiding this comment.
We're aiming to be able to run as a VSCode Web Extesion (#1) so using node-js specific APIs should be avoided.
From https://code.visualstudio.com/api/extension-guides/web-extensions#web-extension-main-file
Node.js globals and libraries such as process, os, setImmediate, path, util, url are not available at runtime.
There was a problem hiding this comment.
Thanks for this code - it's super helpful for building a tree view for this extension.
I really like the UI code, but I have some issues with how the tree view is built:
- It uses
child_processandfs, which are unavailable in web extensions - Parsing is incompatible with Gherkin's I18n
- A lot of this could be implemented in @cucumber/language-service and be reusable in many editors
I wonder if TypeHierarchy might be a possible way to implement this. Or possibly through an extension to the LSP like https://github.com/scalameta/metals-languageclient seems to have done.
|
|
||
| ## Tree View | ||
|
|
||
| I gave it a try and implemented my tree view example into the cucumber extension. In general it does work well, but there are some unknowns which I don't know by now. All my local tests I did with cucumber cpp in combination with my example (just pull it and build it): https://github.com/ThoSe1990/cucumber_example |
There was a problem hiding this comment.
It's interesting to read about your approach, but I think it would be better if you moved this content to the pull request description.
The purpose of the README is to give users a showcase of what this extension does. They are not interested in the details about how a particular feature has been implemented.
It's great to have screenshots!
| @@ -0,0 +1,414 @@ | |||
|
|
|||
| import { spawn } from 'child_process'; | |||
| import * as fs from 'fs'; | |||
There was a problem hiding this comment.
For filesystem access we hide implementation details behind a facade - VscodeFiles.
Please extend this class instead.
|
|
||
| export class tree_view implements vscode.TreeDataProvider<tree_item> | ||
| { | ||
| private data : tree_view_data = new tree_view_data(); |
There was a problem hiding this comment.
Please use camelCase for variables and functions.
|
|
||
| readonly onDidChangeTreeData? : vscode.Event<tree_item | undefined> = this.event_emitter.event; | ||
|
|
||
| private readonly regex_feature = new RegExp('(?<=Feature:).*'); |
There was a problem hiding this comment.
We cannot rely on regexps to parse the source code. Gherkin supports 77 languages, and we should use the @cucumber/gherkin library to parse Gherkin source.
|
This is superseded by cucumber/language-service#98 |
kieran-ryan
added
🐛 bug
🤔 What's changed?
Implement a tree view
⚡️ What's your motivation?
Fix #94
🏷️ What kind of change is this?
♻️ Anything particular you want feedback on?
📋 Checklist:
This text was originally generated from a template, then edited by hand. You can modify the template here.