Skip to content
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

Fix #83 : Enable vuejs support with vue-language-server #210

Closed
wants to merge 1 commit into from

Conversation

jabby
Copy link
Contributor

@jabby jabby commented Jul 30, 2019

Signed-off-by: Gautier de Saint Martin Lacaze gautier.desaintmartinlacaze@gmail.com

@jabby jabby mentioned this pull request Jul 30, 2019
@jabby
Copy link
Contributor Author

jabby commented Jul 30, 2019

I need to fill a lot of CQs.

If I understand, I should do the following CQs. One for each files here:

  • create mode 100644 org.eclipse.wildwebdeveloper/grammars/coffeescript/coffeescript.tmLanguage.json
  • create mode 100644 org.eclipse.wildwebdeveloper/grammars/markdown/markdown.tmLanguage.json
  • create mode 100644 org.eclipse.wildwebdeveloper/grammars/pug/directives.YAML
  • create mode 100644 org.eclipse.wildwebdeveloper/grammars/pug/directives.tmLanguage.json
  • create mode 100644 org.eclipse.wildwebdeveloper/grammars/pug/interpolations.YAML
  • create mode 100644 org.eclipse.wildwebdeveloper/grammars/pug/interpolations.tmLanguage.json
  • create mode 100644 org.eclipse.wildwebdeveloper/grammars/pug/pug.tmLanguage.json
  • create mode 100644 org.eclipse.wildwebdeveloper/grammars/vue/vue-generated.json
  • create mode 100644 org.eclipse.wildwebdeveloper/grammars/vue/vue-html.YAML
  • create mode 100644 org.eclipse.wildwebdeveloper/grammars/vue/vue-html.tmLanguage.json
  • create mode 100644 org.eclipse.wildwebdeveloper/grammars/vue/vue-postcss.json
  • create mode 100644 org.eclipse.wildwebdeveloper/grammars/vue/vue.tmLanguage.json
  • create mode 100644 org.eclipse.wildwebdeveloper/grammars/vue/vue.yaml
  • create mode 100644 org.eclipse.wildwebdeveloper/language-configurations/coffeescript/language-configuration.json
  • create mode 100644 org.eclipse.wildwebdeveloper/language-configurations/markdown/language-configuration.json
  • create mode 100644 org.eclipse.wildwebdeveloper/language-configurations/pug/language-configuration.json
  • create mode 100644 org.eclipse.wildwebdeveloper/language-configurations/vue/vue-default-config.json
  • create mode 100644 org.eclipse.wildwebdeveloper/language-configurations/vue/vue-html-language-configuration.json
  • create mode 100644 org.eclipse.wildwebdeveloper/language-configurations/vue/vue-language-configuration.json
  • create mode 100644 org.eclipse.wildwebdeveloper/language-configurations/vue/vue-postcss-language-configuration.json
  • create mode 100644 org.eclipse.wildwebdeveloper/language-configurations/vue/vue-pug-language-configuration.json

One for the language server. I will upload node_modules folder.

Am I right @mickaelistria ?

@jabby jabby requested a review from mickaelistria July 30, 2019 15:10
@mickaelistria
Copy link
Contributor

Do you really need markdown, coffeescript as part of this change? If not, it would be better to separate them in a separate commit (I'm actually unsure about adding support for languages if there is no decent editor -aka language server- for them).
Sticking to the minimal necessary content and avoiding "noise" will allow easier IP tracking and faster resolution overall.

Then, once we have the minimal set of dependencies, we can open CQs.
For the grammars, if they do all come from the same Git repository/project, they can be submitted together in the same CQ.
For the language server, you can submit a CQ for the LS + dependencies and attach a zip containing the package.json, package-lock.json and node_modules/

@jabby
Copy link
Contributor Author

jabby commented Jul 30, 2019

In vue files (.vue) you can use different script languages. JavaScript is the primary one but you can choose TypeScript or CoffeeScript. So we need this file.

About markdown grammar, the vue grammar declare a dependency on markdown grammar. More over, we can use markdown documentation into typescript code. So I think it's a good idea to add the markdown grammar.

I will fill all CQs in few days (Hopefully tomorrow). I am actually in a train with a bad network connection.

@mickaelistria
Copy link
Contributor

In vue files (.vue) you can use different script languages. JavaScript is the primary one but you can choose TypeScript or CoffeeScript. So we need this file.

Ok. However if those scripts are "inlined", I don't think we need to declare content-types and associations.

@jabby
Copy link
Contributor Author

jabby commented Jul 30, 2019

I also need to add some tests for the new language server.

@jabby
Copy link
Contributor Author

jabby commented Jul 30, 2019

In vue files (.vue) you can use different script languages. JavaScript is the primary one but you can choose TypeScript or CoffeeScript. So we need this file.

Ok. However if those scripts are "inlined", I don't think we need to declare content-types and associations.

I will try with and without content type declaration. If it works without, I will clean plugin.xml

@jabby
Copy link
Contributor Author

jabby commented Aug 26, 2019

I removed content-type for markdown and coffeescript. I also change the node dependency management to comply with your previous work.
I add a simple test for vue-language-server.

I will fill CQs in few minutes.

@jabby
Copy link
Contributor Author

jabby commented Aug 26, 2019

Copy link
Contributor

@mickaelistria mickaelistria left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments in code.
Also, please try tweaking the Jenkinsfile to give more heap to Maven.

org.eclipse.wildwebdeveloper/plugin.xml Outdated Show resolved Hide resolved
org.eclipse.wildwebdeveloper/plugin.xml Outdated Show resolved Hide resolved
@jabby
Copy link
Contributor Author

jabby commented Aug 26, 2019

About files from vscode repository. Should we create another CQ? I saw a CQ for subparts of vscode 1.37.1.

…age-server

Signed-off-by: Gautier de Saint Martin Lacaze <gautier.desaintmartinlacaze@gmail.com>
@jabby
Copy link
Contributor Author

jabby commented Aug 26, 2019

Other question about tweaking the Jenkinsfile. I'm not familiar with this. Should we increase memory with MAVEN_OPTS environment variable?

@mickaelistria
Copy link
Contributor

About files from vscode repository. Should we create another CQ? I saw a CQ for subparts of vscode 1.37.1.

Yes, the subparts only include the language servers. You'll need a separate on for the TextMate and language-configuration files.

Other question about tweaking the Jenkinsfile. I'm not familiar with this. Should we increase memory with MAVEN_OPTS environment variable?

Yes, I think it's the best approach.

@mickaelistria
Copy link
Contributor

@jabby Is there anything blocking progress on this topic?

@jabby
Copy link
Contributor Author

jabby commented Sep 11, 2019

Yes there is a problem with the following CQ https://dev.eclipse.org/ipzilla/show_bug.cgi?id=20748
I need to investigate.

I also need to create another CQ for part of vscode (some of textmate grammar)

@mickaelistria
Copy link
Contributor

Ok, so it's "only" IP and there is no remaining technical challenge?

@davydnorris
Copy link

I was just about to ask how I would go about adding Vuejs support! If there's anything I can do to help here I would be very interested.

@jabby
Copy link
Contributor Author

jabby commented Oct 11, 2019

Hi @davydnorris
Actually the work is blocked. In fact we have a license issue. To support vuejs, we use vue-language-server. But there is a transitive dependency under GPL-3.0.

It's a problem for us but I'm not sure it's a problem for vue-language-server which is under MIT License.

I just created 2 issues about the license of stylint:

@davydnorris
Copy link

Hey @jabby - have you had any luck with this at all?

@davydnorris
Copy link

Hi all,

Happy Western and Eastern NY! Any progress on this blockage at all?

@davydnorris
Copy link

davydnorris commented May 12, 2020

Hi all,

I received notification from one of the blocking projects that their end has now been resolved - does this mean we are closer to being able to merge this pull request? It would be awesome to have Vue.js support - really like this plug-in!!

@jabby jabby marked this pull request as draft June 11, 2020 07:42
@mickaelistria
Copy link
Contributor

does this mean we are closer to being able to merge this pull request?

Yes, probably, Please consider updating the PR to use latest version of dependencies, which should then use the fixed libraries. If this works and doesn't cause regression, we can trigger a further license/IP audit and if result is positive, we can merge.

@davydnorris
Copy link

@jabby - could be good news???! This is your pull request - any chance you can update as above?

@mickaelistria
Copy link
Contributor

We're stuck from IP perspective. I suggest this work should happen in a separate project, made available via Eclipse Marketplace (and associated with .vue files).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants