You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "haidubogdan (via GitHub)" <gi...@apache.org> on 2025/04/07 05:06:22 UTC
[PR] vue files - include parametric embedding support for javascript and styles [netbeans]
haidubogdan opened a new pull request, #8401:
URL: https://github.com/apache/netbeans/pull/8401
This pull request will include updates for embedding support for vue files.
- javscript
- pug | jade
- scss style language
- less style language
For the moment, I didn't manage to include embedding for **typescript**.
The solution might not be that straightforward, as it doesn't have a standard lexer.
---
**^Add meaningful description above**
<details open>
<summary>Click to collapse/expand PR instructions</summary>
By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -
- are all your own work, and you have the right to contribute them.
- are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).
Please make sure (eg. `git log`) that all commits have a valid name and email address for you in the Author field.
If you're a first time contributor, see the Contributing guidelines for more information.
If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.
### PR approval and merge checklist:
1. [ ] Was this PR [correctly labeled](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=240884239#PRsandYouAreviewerGuide-PRtriggeredCIJobs(conditionalCIpipeline)), did the right tests run? When did they run?
2. [ ] Is this PR [squashed](https://cwiki.apache.org/confluence/display/NETBEANS/git%3A+squash+and+merge)?
3. [ ] Are author name / email address correct? Are [co-authors](https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors#creating-co-authored-commits-on-the-command-line) correctly listed? Do the commit messages need updates?
3. [ ] Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?
If this PR targets the delivery branch: [don't merge](https://cwiki.apache.org/confluence/display/NETBEANS/Pull+requests+for+delivery). ([full wiki article](https://cwiki.apache.org/confluence/display/NETBEANS/PRs+and+You+-+A+reviewer+Guide))
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#issuecomment-2946132877
@matthiasblaesing
Beside the decoupling code,
I've removed the css prep and jade dependency from the project, but kept them on test level.
I've added a simple lexer test for pug script.
There was a typo variable on script language lexer state, which I fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#issuecomment-2945856832
> @haidubogdan thank you! Looks like a nice improvement. One final adjustment request. Would you please have a look here:
>
> * https://github.com/matthiasblaesing/netbeans/tree/pr-8401
> * [4bd90af](https://github.com/apache/netbeans/commit/4bd90afc6b3b9cbc79f8ae6d40424970182acfda)
>
> There is no need to open the `css.prep` and `javascript2.jade` modules. You only want to access the language and for that you can use the lexer api to load them at runtime based on mimetype. The new unittest ensures, that the languages don't go away silently.
>
> Would you check the change and see if you can integrate it?
Yes, looks like a good simplification.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2127513395
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/lexer/VueTokenId.java:
##########
@@ -37,8 +40,12 @@
public enum VueTokenId implements TokenId {
HTML("html"), // NOI18N
CSS("css"), // NOI18N
+ STYLE_SCSS("css"), // NOI18N
+ STYLE_LESS("css"), // NOI18N
Review Comment:
@matthiasblaesing you are right, the **lang** logic could use some thought on scaling.
Recently I found about `stylus` preprocessor syntax.
So maybe a refactor could have used a dynamic approach.
Set in the tokenId primaryCategory the expected mime ex: (text/stylus)
In the embedding function
```
protected LanguageEmbedding<? extends TokenId> embedding(
String mime = tokenId.primaryCategory()
language = (Language<? extends TokenId>) provider.findLanguage(mime );
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2106138486
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/lexer/VueTokenId.java:
##########
@@ -37,8 +40,12 @@
public enum VueTokenId implements TokenId {
HTML("html"), // NOI18N
CSS("css"), // NOI18N
+ STYLE_SCSS("css"), // NOI18N
+ STYLE_LESS("css"), // NOI18N
Review Comment:
It's the easiest approach to assign embedding on the tokens.
It's mostly a vue template syntax, so it's also hard to see a generic implementation on this part, and for the moment I do rely on what coloring lexer exists in netbeans platform.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2059238243
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/embedding/VueHtmlEmbeddingProvider.java:
##########
@@ -61,12 +62,16 @@ public List<Embedding> getEmbeddings(final Snapshot snapshot) {
if (id.equals(VueTokenId.HTML)) {
embeddings.add(snapshot.create(ts.offset(), t.length(), TARGET_MIME_TYPE));
+ } else if (t.text() != null) {
+ //issue with bracematcher
+ String fake = new String(new char[t.text().toString().length()]).replace("\0", FILLER); //NOI18N
+ embeddings.add(snapshot.create(fake, TARGET_MIME_TYPE));
Review Comment:
Yes, here is an example.
I see that it's mostly for javascript **script** tag.
Maybe a mimepath bug also?
It seems to disturb the closing tag matcher position also.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2059238243
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/embedding/VueHtmlEmbeddingProvider.java:
##########
@@ -61,12 +62,16 @@ public List<Embedding> getEmbeddings(final Snapshot snapshot) {
if (id.equals(VueTokenId.HTML)) {
embeddings.add(snapshot.create(ts.offset(), t.length(), TARGET_MIME_TYPE));
+ } else if (t.text() != null) {
+ //issue with bracematcher
+ String fake = new String(new char[t.text().toString().length()]).replace("\0", FILLER); //NOI18N
+ embeddings.add(snapshot.create(fake, TARGET_MIME_TYPE));
Review Comment:
Yes, here is an example.
I see that it's mostly for javascript **script** tag.
Maybe a mimepath bug also?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2062502712
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/lexer/VueLexer.java:
##########
@@ -29,6 +29,13 @@
* @author bogdan.haidu
*/
public class VueLexer extends AbstractAntlrLexerBridge<VueAntlrColoringLexer, VueTokenId> {
+ public static final String PUG_LANG = "pug"; //NOI18N
+ public static final String JADE_LANG = "jade"; //NOI18N
+
+ //Styles
+ //embedding not working in netbeans ide
+ public static final String SCSS_LANG = "scss"; //NOI18N
+ public static final String LESS_LANG = "less"; //NOI18N
Review Comment:
Maybe I left the comment in a work in progress moment.
The token separation is useful for the embedding process.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2127532107
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/embedding/VueHtmlEmbeddingProvider.java:
##########
@@ -56,17 +61,26 @@ public List<Embedding> getEmbeddings(final Snapshot snapshot) {
try {
while (ts.moveNext()) {
- Token<?> t = ts.token();
- TokenId id = t.id();
+ Token<?> token = ts.token();
+ TokenId id = token.id();
- if (id.equals(VueTokenId.HTML)) {
- embeddings.add(snapshot.create(ts.offset(), t.length(), TARGET_MIME_TYPE));
+ if (token.text() != null && id instanceof VueTokenId) {
+ switch ((VueTokenId) id) {
+ //javascript, css, scss, less require content embedding due to braces matcher issue
+ case HTML, JAVASCRIPT, CSS -> {
+ embeddings.add(snapshot.create(ts.offset(), token.length(), TARGET_MIME_TYPE));
+ }
+ case STYLE_SCSS, STYLE_LESS -> {
+ String fake = new String(new char[token.text().length()]).replace("\0", FILLER); //NOI18N
+ embeddings.add(snapshot.create(fake, TARGET_MIME_TYPE));
+ }
+ }
Review Comment:
Added your update proposal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#issuecomment-2955409287
@matthiasblaesing
I've re-added the dependency for css prep and jade modules.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing merged PR #8401:
URL: https://github.com/apache/netbeans/pull/8401
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2127514933
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/grammar/antlr4/coloring/ColoringLexerAdaptor.java:
##########
@@ -76,4 +61,43 @@ public void setVarInterpolationOpened(boolean state) {
public boolean isVarInterpolationOpened() {
return varInterpolationOpened;
}
+
+ public void setScriptLanguage(String lang) {
+ scriptLanguage = lang;
+ }
+
+ public void setScriptLanguage() {
+ scriptLanguage = extractLangFromInput();
+ }
+
+ public String getScriptLanguage() {
+ return scriptLanguage;
+ }
+
+ public void setStyleLanguage(String lang) {
+ scriptLanguage = lang;
+ }
+
+ public void setStyleLanguage() {
+ styleLanguage = extractLangFromInput();
+ }
+
+ public String getStyleLanguage() {
+ return styleLanguage;
+ }
+
+ private String extractLangFromInput() {
+ String input = this.getText();
+ String langAttrEq = LANG_ATTR + "="; //NOI18N
+ if (input == null || !input.startsWith(langAttrEq)) {
+ return null;
+ }
+ String langValue = input.substring(langAttrEq.length());
+ //check if only quotes
+ if (langValue.length() == 2) {
+ return null;
+ }
+
+ return langValue.substring(1, langValue.length() - 1);
Review Comment:
As it is specific to vue syntax, I think it should be ok.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#issuecomment-2945521243
@haidubogdan thank you! Looks like a nice improvement. One final adjustment request. Would you please have a look here:
- https://github.com/matthiasblaesing/netbeans/tree/pr-8401
- https://github.com/apache/netbeans/commit/4bd90afc6b3b9cbc79f8ae6d40424970182acfda
There is no need to open the `css.prep` and `javascript2.jade` modules. You only want to access the language and for that you can use the lexer api to load them at runtime based on mimetype. The new unittest ensures, that the languages don't go away silently.
Would you check the change and see if you can integrate it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#issuecomment-2950571212
> @matthiasblaesing Beside the decoupling code, I've removed the css prep and jade dependency from the project, but kept them on test level.
This must be reverted. The dependency is required, so that when `javascript2.vue` module is loaded, `css.prop` and `javascript2.jade` are made available. If not some other module loads these two modules (or they are auto activating), then `Language.find` will not be able to resolve the language and you will run into problems, because you pass a `null` language to `LanguageEmbedding.create`. Adding the too modules as test modules essentially invalidates the unittest, as now testing and runtime test different environments.
The are two levels of coupling here:
- General module dependency: required, as we assume the target languages are present. We are not interested in the internals, but need the modules to be available.
- Implementation/code dependency: We don't need access to the source code. It was only ever used to load the language and usage can be substituted.
>I've added a simple lexer test for pug script. There was a typo variable on script language lexer state, which I fixed.
Makes sense.
Looking at the github diff, the revert of the changes to `webcommon/javascript2.jade/nbproject/project.xml` is incomplete, there are still whitespace changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2062386004
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/embedding/VueHtmlEmbeddingProvider.java:
##########
@@ -61,12 +62,16 @@ public List<Embedding> getEmbeddings(final Snapshot snapshot) {
if (id.equals(VueTokenId.HTML)) {
embeddings.add(snapshot.create(ts.offset(), t.length(), TARGET_MIME_TYPE));
+ } else if (t.text() != null) {
+ //issue with bracematcher
+ String fake = new String(new char[t.text().toString().length()]).replace("\0", FILLER); //NOI18N
+ embeddings.add(snapshot.create(fake, TARGET_MIME_TYPE));
Review Comment:
The script part is easy to fix but for :
Seems to be a issue with the fact that there is no offset check.
Unfortunately the issue is already in the vue implementation from nb 26-rc1, but somehow I didn't notice it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2083597159
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/embedding/VueHtmlEmbeddingProvider.java:
##########
@@ -56,17 +61,26 @@ public List<Embedding> getEmbeddings(final Snapshot snapshot) {
try {
while (ts.moveNext()) {
- Token<?> t = ts.token();
- TokenId id = t.id();
+ Token<?> token = ts.token();
+ TokenId id = token.id();
- if (id.equals(VueTokenId.HTML)) {
- embeddings.add(snapshot.create(ts.offset(), t.length(), TARGET_MIME_TYPE));
+ if (token.text() != null && id instanceof VueTokenId) {
+ switch ((VueTokenId) id) {
+ //javascript, css, scss, less require content embedding due to braces matcher issue
+ case HTML, JAVASCRIPT, CSS -> {
+ embeddings.add(snapshot.create(ts.offset(), token.length(), TARGET_MIME_TYPE));
+ }
+ case STYLE_SCSS, STYLE_LESS -> {
+ String fake = new String(new char[token.text().length()]).replace("\0", FILLER); //NOI18N
+ embeddings.add(snapshot.create(fake, TARGET_MIME_TYPE));
+ }
+ }
Review Comment:
This looks strange. The VueHtmlEmbeddingProvider should provide embedding for HTML. The other embeddings should be handled/and are being handled by their respecitve counter parts.
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/embedding/VueJsEmbeddingProvider.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.javascript2.vue.editor.embedding;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import org.netbeans.api.lexer.Token;
+import org.netbeans.api.lexer.TokenHierarchy;
+import org.netbeans.api.lexer.TokenId;
+import org.netbeans.api.lexer.TokenSequence;
+import org.netbeans.modules.javascript2.vue.editor.VueLanguage;
+import static org.netbeans.modules.javascript2.vue.editor.embedding.VueJsEmbeddingProvider.TARGET_MIME_TYPE;
+import org.netbeans.modules.javascript2.vue.editor.lexer.VueTokenId;
+import org.netbeans.modules.parsing.api.Embedding;
+import org.netbeans.modules.parsing.api.Snapshot;
+import org.netbeans.modules.parsing.spi.EmbeddingProvider;
+
+/**
+ *
+ *
+ * @author bhaidu
+ */
Review Comment:
Please remove. The author information is available from git history.
##########
ide/css.lib/src/org/netbeans/modules/css/lib/nbparser/CssParser.java:
##########
@@ -81,24 +81,40 @@ public void parse(Snapshot snapshot, Task task, SourceModificationEvent event) t
if (snapshot == null) {
return;
}
-
+
this.snapshot = snapshot;
FileObject fo = snapshot.getSource().getFileObject();
- String fileName = fo == null ? "no file" : fo.getPath(); //NOI18N
- String mimeType = topLevelSnapshotMimetype != null ? topLevelSnapshotMimetype : (fo == null ? null : fo.getMIMEType());
+ String fileName;
+ String mimeType;
+ if (fo != null) {
+ fileName= fo.getPath();
+ MimePath mimePath = snapshot.getMimePath();
+
+ //scss, less mime embedding is a top layer for text/css
+ //positioned at the penultimate position
+ if (mimePath != null && mimePath.size() > 2) {
+ mimeType = mimePath.getMimeType(mimePath.size() - 2);
+ } else {
+ mimeType = fo.getMIMEType();
+ }
+ } else {
+ fileName = "no file";
+ mimeType = topLevelSnapshotMimetype;
+ }
Review Comment:
```suggestion
if (topLevelSnapshotMimetype == null) {
fileName= fo.getPath();
MimePath mimePath = snapshot.getMimePath();
//scss, less mime embedding is a top layer for text/css
//positioned at the penultimate position
if (mimePath != null && mimePath.size() > 2) {
mimeType = mimePath.getMimeType(mimePath.size() - 2);
} else {
mimeType = snapshot.getMimeType();
}
} else {
fileName = "no file";
mimeType = topLevelSnapshotMimetype;
}
```
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/grammar/antlr4/coloring/VueAntlrColoringLexer.g4:
##########
@@ -85,16 +100,34 @@ SCRIPT_ATTR_QUOTE : '"' {this.setAttrQuoteState(true);}->type(QUOTE_ATTR);
SCRIPT_ATTR_OTHER : . ->type(JAVASCRIPT_ATTR);
EXIT_SCRIPT_ATTR_EOF : EOF->type(HTML),popMode;
+mode INSIDE_STYLE_TAG_START;
+
+STYLE_LANG_ATTR : 'lang=' StringLiteral {this.setStyleLanguage();}->type(HTML);
Review Comment:
see comment about HTML5 structure above.
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/lexer/VueTokenId.java:
##########
@@ -37,8 +40,12 @@
public enum VueTokenId implements TokenId {
HTML("html"), // NOI18N
CSS("css"), // NOI18N
+ STYLE_SCSS("css"), // NOI18N
+ STYLE_LESS("css"), // NOI18N
Review Comment:
Will this scale? Webdevelopers have the tendency to reinvent the wheel (LESS, SCSS, SASS and now also JS variants).
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/grammar/antlr4/coloring/ColoringLexerAdaptor.java:
##########
@@ -76,4 +61,43 @@ public void setVarInterpolationOpened(boolean state) {
public boolean isVarInterpolationOpened() {
return varInterpolationOpened;
}
+
+ public void setScriptLanguage(String lang) {
+ scriptLanguage = lang;
+ }
+
+ public void setScriptLanguage() {
+ scriptLanguage = extractLangFromInput();
+ }
+
+ public String getScriptLanguage() {
+ return scriptLanguage;
+ }
+
+ public void setStyleLanguage(String lang) {
+ scriptLanguage = lang;
+ }
+
+ public void setStyleLanguage() {
+ styleLanguage = extractLangFromInput();
+ }
+
+ public String getStyleLanguage() {
+ return styleLanguage;
+ }
+
+ private String extractLangFromInput() {
+ String input = this.getText();
+ String langAttrEq = LANG_ATTR + "="; //NOI18N
+ if (input == null || !input.startsWith(langAttrEq)) {
+ return null;
+ }
+ String langValue = input.substring(langAttrEq.length());
+ //check if only quotes
+ if (langValue.length() == 2) {
+ return null;
+ }
+
+ return langValue.substring(1, langValue.length() - 1);
Review Comment:
Is this safe? HTML allows unquoted attributes and there maybe spaces between attribute, equals sign and value. Is the same true for vue?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#issuecomment-2941352525
Sorry @matthiasblaesing I got sidetracked with a personal development.
And also I studied using HtmlPlugin : https://github.com/apache/netbeans/compare/master...haidubogdan:netbeans:vue_html_plugin_impl (could be a interesting approach)
I will analyze your code proposal and try to resolve the code reviews on the current branch in order to be able to merge.
Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2127530795
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/grammar/antlr4/coloring/VueAntlrColoringLexer.g4:
##########
@@ -85,16 +100,34 @@ SCRIPT_ATTR_QUOTE : '"' {this.setAttrQuoteState(true);}->type(QUOTE_ATTR);
SCRIPT_ATTR_OTHER : . ->type(JAVASCRIPT_ATTR);
EXIT_SCRIPT_ATTR_EOF : EOF->type(HTML),popMode;
+mode INSIDE_STYLE_TAG_START;
+
+STYLE_LANG_ATTR : 'lang=' StringLiteral {this.setStyleLanguage();}->type(HTML);
Review Comment:
Same reasoning. Today in pure html the lang attribute usually identifies the language of a text (en, es, it ...).
But in vue it's used to identify pre-processors language for an element like script, style. So I think it should be safe.
I see that vue has some interesting lint rules for lang blocks.
https://eslint.vuejs.org/rules/block-lang
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2052730951
##########
ide/css.lib/src/org/netbeans/modules/css/lib/nbparser/CssParser.java:
##########
@@ -93,12 +93,23 @@ public void parse(Snapshot snapshot, Task task, SourceModificationEvent event) t
//parse just an empty string in case of an oversize snapshot
CharSequence source = tooLargeSnapshot ? "" : snapshot.getText();
-
+
+ String mimePath = snapshot.getMimePath() != null ? snapshot.getMimePath().getPath() : null;
+ if (mimePath != null) {
+ //vue plugin embedding
+ if (mimePath.startsWith("text/x-vue/text/scss")) { //NOI18N
+ mimeType = "text/scss"; //NOI18N
+ } else if (mimePath.startsWith("text/x-vue/text/less")){ //NOI18N
+ mimeType = "text/less"; //NOI18N
+ }
+ }
+
Review Comment:
I think we should fix the CSS parsing correctly. Currently CSS/SCSS/LESS switching is a mess as it relies on three different patterns. Before this change:
- for tests the mimetype is forced via the constructor `org.netbeans.modules.css.lib.nbparser.CssParser.CssParser(String)`
- for "plain" stylesheets it is deduced from the file mimetype
- for all other types parsing falls hard to `text/css`
What I envision (please see if want to pursue this, else I'll create a followup) is, that we drop the specialcases like this:
- check the inner most element of the mimepath (`snapshot.getMimePath().getMimeType(snapshot.getMimePath().size() - 1)`)
- if that is from the set `text/less`, `text/scss` or `text/css` use it, else fall back to `text/css`
For unittests we should be able to create a correct snapshot, for normal execution, the behaviour should match.
That way we don't need to special case CSS/SCSS/LESS embeddings in other languages and the new vue-specialcase is not necessary.
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/resources/vue16.png:
##########
Review Comment:
Nicer. I asumme you are still the author?
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/embedding/VueHtmlEmbeddingProvider.java:
##########
@@ -33,21 +33,22 @@
/**
* this will enable braces matches of html elements
- *
+ *
* @author bhaidu
*/
@EmbeddingProvider.Registration(
mimeType = VueLanguage.MIME_TYPE,
targetMimeType = "text/html")
Review Comment:
Replace with `TARGET_MIME_TYPE` (as done in other place)
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/embedding/VueHtmlEmbeddingProvider.java:
##########
@@ -61,12 +62,16 @@ public List<Embedding> getEmbeddings(final Snapshot snapshot) {
if (id.equals(VueTokenId.HTML)) {
embeddings.add(snapshot.create(ts.offset(), t.length(), TARGET_MIME_TYPE));
+ } else if (t.text() != null) {
+ //issue with bracematcher
+ String fake = new String(new char[t.text().toString().length()]).replace("\0", FILLER); //NOI18N
Review Comment:
Please check if this works:
```suggestion
String fake = new String(new char[t.length()]).replace("\0", FILLER); //NOI18N
```
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/embedding/VueHtmlEmbeddingProvider.java:
##########
@@ -61,12 +62,16 @@ public List<Embedding> getEmbeddings(final Snapshot snapshot) {
if (id.equals(VueTokenId.HTML)) {
embeddings.add(snapshot.create(ts.offset(), t.length(), TARGET_MIME_TYPE));
+ } else if (t.text() != null) {
+ //issue with bracematcher
+ String fake = new String(new char[t.text().toString().length()]).replace("\0", FILLER); //NOI18N
+ embeddings.add(snapshot.create(fake, TARGET_MIME_TYPE));
Review Comment:
It would be great to have an example what goes wrong here. I have an idea, but like to verify. Thank you.
##########
webcommon/javascript2.vue/test/unit/data/testfiles/embedding/vue_embedding_01.vue:
##########
@@ -0,0 +1,51 @@
+<!--
Review Comment:
For tests please include the ASF header. There is no need to skip it and we should carry it where possible. This comment applies for other test files too.
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/lexer/VueLexer.java:
##########
@@ -29,6 +29,13 @@
* @author bogdan.haidu
*/
public class VueLexer extends AbstractAntlrLexerBridge<VueAntlrColoringLexer, VueTokenId> {
+ public static final String PUG_LANG = "pug"; //NOI18N
+ public static final String JADE_LANG = "jade"; //NOI18N
+
+ //Styles
+ //embedding not working in netbeans ide
+ public static final String SCSS_LANG = "scss"; //NOI18N
+ public static final String LESS_LANG = "less"; //NOI18N
Review Comment:
With the suggested changes for `CssParser` it should be possible to create a mimepath `text/x-vue/text/scss`, that will be properly handled. This might reveals broken assumptions in the CSS parser, but then we should fix them ;-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#issuecomment-2907715287
@matthiasblaesing , @mbien as a work in progress info I will remove some embeddings implementation in favor of a custom lexer plugin as we have with angular plugin.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#issuecomment-2941117472
@haidubogdan I had another quick look and I noticed, that JS integration did not work properly. I fixed it be cutting down the `VueHtmlEmbeddingProvider` to the scope it should handle (in my first pass I already left an inline comment, that it looked strange). With this code:
```java
@Override
public List<Embedding> getEmbeddings(final Snapshot snapshot) {
TokenHierarchy<?> tokenHierarchy = snapshot.getTokenHierarchy();
TokenSequence<?> ts = tokenHierarchy.tokenSequence();
if (ts == null || !ts.isValid()) {
return Collections.emptyList();
}
ts.moveStart();
List<Embedding> embeddings = new ArrayList<>();
try {
while (ts.moveNext()) {
Token<?> token = ts.token();
TokenId id = token.id();
if (token.text() != null && id == VueTokenId.HTML) {
embeddings.add(snapshot.create(ts.offset(), token.length(), TARGET_MIME_TYPE));
}
}
} catch (Exception ex) {
return Collections.emptyList();
}
if (embeddings.isEmpty()) {
return Collections.emptyList();
} else {
return Collections.singletonList(Embedding.create(embeddings));
}
}
```
Opening `component_01.vue` seems to work correctly. The JS highlighting and code completion worked. The same is true for the style tag.
So why not wrap this up and make it a mergable change, so that we can get it in?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2053430395
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/resources/vue16.png:
##########
Review Comment:
Yes, it's my creation :) .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2062509903
##########
ide/css.lib/src/org/netbeans/modules/css/lib/nbparser/CssParser.java:
##########
@@ -93,12 +93,23 @@ public void parse(Snapshot snapshot, Task task, SourceModificationEvent event) t
//parse just an empty string in case of an oversize snapshot
CharSequence source = tooLargeSnapshot ? "" : snapshot.getText();
-
+
+ String mimePath = snapshot.getMimePath() != null ? snapshot.getMimePath().getPath() : null;
+ if (mimePath != null) {
+ //vue plugin embedding
+ if (mimePath.startsWith("text/x-vue/text/scss")) { //NOI18N
+ mimeType = "text/scss"; //NOI18N
+ } else if (mimePath.startsWith("text/x-vue/text/less")){ //NOI18N
+ mimeType = "text/less"; //NOI18N
+ }
+ }
+
Review Comment:
I discovered that I had to use `snapshot.getMimePath().getMimeType(snapshot.getMimePath().size() - 2)` for the penultimate mimeType.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2061546075
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/embedding/VueHtmlEmbeddingProvider.java:
##########
@@ -61,12 +62,16 @@ public List<Embedding> getEmbeddings(final Snapshot snapshot) {
if (id.equals(VueTokenId.HTML)) {
embeddings.add(snapshot.create(ts.offset(), t.length(), TARGET_MIME_TYPE));
+ } else if (t.text() != null) {
+ //issue with bracematcher
+ String fake = new String(new char[t.text().toString().length()]).replace("\0", FILLER); //NOI18N
+ embeddings.add(snapshot.create(fake, TARGET_MIME_TYPE));
Review Comment:
Yeah, most probably a matcher bug.... This makes this change specially interesting as it makes it necessary to cleanup the language embeddings.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2127472563
##########
ide/css.lib/src/org/netbeans/modules/css/lib/nbparser/CssParser.java:
##########
@@ -81,24 +81,40 @@ public void parse(Snapshot snapshot, Task task, SourceModificationEvent event) t
if (snapshot == null) {
return;
}
-
+
this.snapshot = snapshot;
FileObject fo = snapshot.getSource().getFileObject();
- String fileName = fo == null ? "no file" : fo.getPath(); //NOI18N
- String mimeType = topLevelSnapshotMimetype != null ? topLevelSnapshotMimetype : (fo == null ? null : fo.getMIMEType());
+ String fileName;
+ String mimeType;
+ if (fo != null) {
+ fileName= fo.getPath();
+ MimePath mimePath = snapshot.getMimePath();
+
+ //scss, less mime embedding is a top layer for text/css
+ //positioned at the penultimate position
+ if (mimePath != null && mimePath.size() > 2) {
+ mimeType = mimePath.getMimeType(mimePath.size() - 2);
+ } else {
+ mimeType = fo.getMIMEType();
+ }
+ } else {
+ fileName = "no file";
+ mimeType = topLevelSnapshotMimetype;
+ }
Review Comment:
It's a tricky situation with the proposed code as it will fail on tests.
if (topLevelSnapshotMimetype == null) {
fileName= fo.getPath();
and adding `if (topLevelSnapshotMimetype == null && fo != null) { `
is more or less a different version of the refactor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2103191925
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/VueLanguage.java:
##########
@@ -93,6 +93,13 @@ public Parser getParser() {
return new VueParser();
}
+ @Override
+ public boolean isIdentifierChar(char c) {
+ return super.isIdentifierChar(c)
+ || c == '-' //NOI18N
+ ;
Review Comment:
this could be one line I think ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2106130988
##########
ide/css.lib/src/org/netbeans/modules/css/lib/nbparser/CssParser.java:
##########
@@ -81,24 +81,40 @@ public void parse(Snapshot snapshot, Task task, SourceModificationEvent event) t
if (snapshot == null) {
return;
}
-
+
this.snapshot = snapshot;
FileObject fo = snapshot.getSource().getFileObject();
- String fileName = fo == null ? "no file" : fo.getPath(); //NOI18N
- String mimeType = topLevelSnapshotMimetype != null ? topLevelSnapshotMimetype : (fo == null ? null : fo.getMIMEType());
+ String fileName;
+ String mimeType;
+ if (fo != null) {
+ fileName= fo.getPath();
+ MimePath mimePath = snapshot.getMimePath();
+
+ //scss, less mime embedding is a top layer for text/css
+ //positioned at the penultimate position
+ if (mimePath != null && mimePath.size() > 2) {
+ mimeType = mimePath.getMimeType(mimePath.size() - 2);
+ } else {
+ mimeType = fo.getMIMEType();
+ }
+ } else {
+ fileName = "no file";
+ mimeType = topLevelSnapshotMimetype;
+ }
Review Comment:
@matthiasblaesing So far I see that the `topLevelSnapshotMimetype` is a flag used in **Test Context** to parse css **Source** Snapshot without a fileObject and also imposing a mimeType.
My observation with the current logic is that there is no real check if a **fo** is null or not, yet it's still possible to use a **Source** object without a **FileObject** and have a null **topLevelSnapshotMimetype** .
So it will have an exception at this part.
```
fileName= fo.getPath();
```
Strangely, the update didn't break any tests, so topLevelSnapshotMimetype must be used for sources without a fileObject. My general idea would be that the mimeType should be obtained correctly from the fileObject.
Maybe a issue could be with the testUtils which creates a common text/css mimeType document to be used for parsing
scss and less files, so the context needs to be created with specific flags.
The coupling scss | less => css mimeType is really confusing here :) .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] vue files - include parametric embedding support for javascript and styles [netbeans]
Posted by "haidubogdan (via GitHub)" <gi...@apache.org>.
haidubogdan commented on code in PR #8401:
URL: https://github.com/apache/netbeans/pull/8401#discussion_r2072607107
##########
webcommon/javascript2.vue/src/org/netbeans/modules/javascript2/vue/editor/embedding/VueHtmlEmbeddingProvider.java:
##########
@@ -61,12 +62,16 @@ public List<Embedding> getEmbeddings(final Snapshot snapshot) {
if (id.equals(VueTokenId.HTML)) {
embeddings.add(snapshot.create(ts.offset(), t.length(), TARGET_MIME_TYPE));
+ } else if (t.text() != null) {
+ //issue with bracematcher
+ String fake = new String(new char[t.text().toString().length()]).replace("\0", FILLER); //NOI18N
+ embeddings.add(snapshot.create(fake, TARGET_MIME_TYPE));
Review Comment:
The brace matcher issue is already exists on NB 26 RC-1. For the moment I will mark this as resolved.
The issue is not clear, but so far what I notice that in the embedding process (`HtmlBracesMatching`) the token '>' is received with the wrong offset. And the token is coming from the TokenHierarchy.
I think I should also look at how HtmlLexerPlugin is implemented for knockout js and Angular.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists