Allow JavaScript Code Hints only in its context #11263

Merged
merged 1 commit into from Jun 22, 2015

Projects

None yet

4 participants

@sprintr
Contributor
sprintr commented Jun 13, 2015

Fixes a regression I made in #11245 where JavaScript code hints were globally available in a PHP document. This will make sure they are only available inside script tags.

@MarcelGerber MarcelGerber added this to the Release 1.4 milestone Jun 13, 2015
@abose abose commented on an outdated diff Jun 14, 2015
src/extensions/default/JavaScriptCodeHints/main.js
@@ -341,10 +341,11 @@ define(function (require, exports, module) {
};
/**
- * @return {boolean} - true if the document is a html file
+ * @return {boolean} - true if the document is a html or php file
*/
function isHTMLFile(document) {
@abose
abose Jun 14, 2015 Contributor

Function name is now misleading. maybe change including usages to isHTMLOrPHPFile(document)
or alternatively move the check to a more localized place.

@abose abose self-assigned this Jun 15, 2015
@abose abose commented on the diff Jun 15, 2015
src/extensions/default/JavaScriptCodeHints/main.js
@@ -56,6 +56,9 @@ define(function (require, exports, module) {
noHintsOnDot = false, // preference setting to prevent hints on dot
ignoreChange; // can ignore next "change" event if true;
+ // Languages that support inline JavaScript
+ var _inlineScriptLanguages = ["html", "php"];
@abose
abose Jun 15, 2015 Contributor

Would it make sense to define this as a preference?

@sprintr
sprintr Jun 15, 2015 Contributor

Don't think so. These are the only two languages that support inline JS.

@sprintr
sprintr Jun 15, 2015 Contributor

In case someone wants to disable JavaScript code hints in PHP, they can use language/php layer to disable them.

@abose
abose Jun 15, 2015 Contributor

jsp/asp I remember can do similar js mixin as PHP.

@sprintr
sprintr Jun 15, 2015 Contributor

@abose jsp/asp files will be highlighted as html files.

@TomMalbran
TomMalbran Jun 15, 2015 Contributor

Maybe is something that could be defined as part of the language in the LanguageManager?

@abose abose merged commit 4f7be02 into adobe:master Jun 22, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abose
Contributor
abose commented Jun 22, 2015

Looks good. Merging.

@sprintr sprintr deleted the sprintr:fix-php-js-hints branch Jun 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment