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

colorize.include is not working #890

Open
reefhound opened this issue Jan 21, 2023 · 29 comments
Open

colorize.include is not working #890

reefhound opened this issue Jan 21, 2023 · 29 comments

Comments

@reefhound
Copy link

reefhound commented Jan 21, 2023

The Colorize tray icon says "Colorize is not activated for this file"

I'm trying to colorize qss files, Python's PyQt styling. I have my settings as:
"colorize.include": [
"**/*.qss"
],

The content is similar to CSS and colorizes fine when pasted into a file with a .css extension so the issue is it's not processing the .qss file. I have not seen one person report getting this to work. Is there some additional step required?

I've tried both the original version by Kami and the newer fork.

Also, the "colorize.exclude" doesn't work either. I added to exclude .css and it still colorizes it even after restart.
"colorize.exclude": ["/*.css", "/*.scss"],

@reefhound
Copy link
Author

reefhound commented Jan 22, 2023

Found the issue. In Extensions.js (In Kami's version, Yuteng's is obfuscated) is this function:

function isIncludedFile(fileName) {
return config.filesToIncludes.find((globPattern) => globToRegexp(globPattern).test(fileName)) !== undefined;
}

Replacing the return to just check for hardcoded extension makes files of that extension work.
function isIncludedFile(fileName) {
return fileName.endsWith(".qss");
)

So there is something whacked with the regex and my regex skills are weak.

@reefhound
Copy link
Author

Nope. Same fail.

@reefhound
Copy link
Author

Besides, the forked version has essentially the same code as the original in the function isIncludedFile.
line 164 of Extensions.js of fork
return config.filesToIncludes.find((globPattern: string) => globToRegexp(globPattern).test(fileName)) !== undefined;

line 149 of Extensions.js of original
return config.filesToIncludes.find((globPattern) => globToRegexp(globPattern).test(fileName)) !== undefined;

tjx666 pushed a commit to tjx666/vscode-colorize that referenced this issue Jan 23, 2023
@tjx666
Copy link

tjx666 commented Jan 23, 2023

@reefhound try latest version

@reefhound
Copy link
Author

@reefhound try latest version

Did you actually try the latest version? You can add a non-Language Supported file pattern in "colorize.include" and get it to activate and colorize?

I have the latest code in the repo. The canColorize function is what determines if the page is activated. It does not even look at "colorize.exclude". In fact, there is a comment that it needs to be added and the include needs to be reworked. And the isIncludedFile function is the same as what I posted in post #2 so it hasn't been changed.

/**

  • Check if the file is the colorize.include setting
  • @param {string} fileName A valid filename (path to the file)
  • @returns {boolean}
    */
    function isIncludedFile(fileName: string): boolean {
    return config.filesToIncludes.find((globPattern: string) => globToRegexp(globPattern).test(fileName)) !== undefined;
    }

/**

  • Check if a file can be colorized by COLORIZE
  • @param {TextDocument} document The document to test
  • @returns {boolean}
    */
    function canColorize(document: TextDocument): boolean { // update to use filesToExcludes. Remove isLanguageSupported ? checking path with file extension or include glob pattern should be enough
    return isLanguageSupported(document.languageId) || isIncludedFile(document.fileName);
    }

@tjx666
Copy link

tjx666 commented Jan 23, 2023

@reefhound
I mean try my latest version. I publish a new version instead of official version.
The official version is not maintained any more.
I fixed it in this commit: tjx666@1f91387

image

@reefhound
Copy link
Author

I'm not installing "open source" that has been obfuscated. Where is the development repo? I'll be able to tell if it's fixed from looking at the code itself.

@reefhound
Copy link
Author

I've already fixed Kami's for myself. Since all I care about is absolute extensions, I only check the extension. I get the extension and do a simple check that the filename ends with the extension. I added an exclude function and allowed an exclude match to short circuit the canColorize function.

@reefhound
Copy link
Author

reefhound commented Jan 23, 2023

/**

  • Check if the file is the colorize.exclude setting
  • @param {string} fileName A valid filename (path to the file)
  • @returns {boolean}
    /
    function isExcludedFile(fileName) {
    var ext = fileName.substring(fileName.lastIndexOf("."), fileName.length) || fileName;
    var res = config.filesToExcludes.find((globPattern) => globPattern.toString().endsWith(ext));
    return res !== undefined;
    }
    /
    *
  • Check if the file is the colorize.include setting
  • @param {string} fileName A valid filename (path to the file)
  • @returns {boolean}
    /
    function isIncludedFile(fileName) {
    var ext = fileName.substring(fileName.lastIndexOf("."), fileName.length) || fileName;
    var res = config.filesToIncludes.find((globPattern) => globPattern.toString().endsWith(ext));
    return res !== undefined;
    }
    /
    *
  • Check if a file can be colorized by COLORIZE
  • @param {TextDocument} document The document to test
  • @returns {boolean}
    */
    function canColorize(document) {
    var res = isExcludedFile(document.fileName);
    if (res) return false;
    return isLanguageSupported(document.languageId) || isIncludedFile(document.fileName);
    }

@reefhound
Copy link
Author

never mind, found your code. Looks like you replaced find with some and added the exclude check inline rather than separate function, I'll test.

@reefhound
Copy link
Author

reefhound commented Jan 23, 2023

I installed your latest and still not working for me.
My settings:
"colorize.include": ["/*.qss"],
"colorize.exclude": ["
/.css", "**/.scss"],
"colorize.colorized_colors": ["BROWSERS_COLORS", "HEXA", "RGB", "HSL"],
"colorize.languages": ["css", "sass", "scss", "less", "xml", "svg"]

My snippet to colorize:
QLineEdit {
background-color:#D8BFD8;
border:2px solid #6272a4;
color:#586796;
}

This snippet, in a filename with an included qss extension, does not activate and colorize yet in an excluded css extension continues to active and colorize.

@reefhound
Copy link
Author

Note that your exclusion check comes after your language check so an excluded file that is language supported will never get excluded. In my fix, I do the exclusion check first.

function isIncludedFile(fileName: string): boolean {
if (config.filesToExcludes.some((globPattern) => globToRegexp(globPattern).test(fileName))) {
return false;
}
return config.filesToIncludes.some((globPattern: string) =>
globToRegexp(globPattern).test(fileName),
);
}

/**

  • Check if a file can be colorized by COLORIZE
  • @param {TextDocument} document The document to test
  • @returns {boolean}
    */
    function canColorize(document: TextDocument): boolean { // update to use filesToExcludes. Remove isLanguageSupported ? checking path with file extension or include glob pattern should be enough
    return isLanguageSupported(document.languageId) || isIncludedFile(document.fileName);
    }

@tjx666
Copy link

tjx666 commented Jan 23, 2023

you need write **/*.qss, not /*.qss

@reefhound
Copy link
Author

reefhound commented Jan 23, 2023

"colorize.include": ["/*.qss"],
"colorize.exclude": ["
/.css", "**/.scss"],

I have it as double asterisk, slash, asterisk extension. It's this discussion board software changing it.

@tjx666
Copy link

tjx666 commented Jan 23, 2023

I will fix the exclude logic tomorrow, I need to sleep now.

@tjx666
Copy link

tjx666 commented Jan 23, 2023

The qss file can be colorized in my local machine, I will show you my settings and screenshot tomorrow

@tjx666
Copy link

tjx666 commented Jan 24, 2023

image

{
    "colorize.languages": ["css", "scss", "less", "xml", "svg"],
    "colorize.enable_search_variables": false,
    "colorize.include": [
        "**/*.css",
        "**/*.scss",
        "**/*.sass",
        "**/*.less",
        "**/*.styl",
        "**/*.qss"
    ],
}

I had publish a new version fix the exclude logic tjx666@6aa0231

@reefhound
Copy link
Author

reefhound commented Jan 24, 2023

Just tried latest, still doesn't work for me. I did a complete uninstall, restart, install cycle. I think something else is going on. The code in the commit looks ok. Are you sure the build pipeline is working right? Have you confirmed it working on your end with the production version? I'm just wondering if the package reg-to-exp is the problem.

I'm using the latest version of Vscode on Windows 10. The extension version shows as 0.12.8

@reefhound
Copy link
Author

reefhound commented Jan 24, 2023

I think you're not using Windows. Could be the way Windows uses backslashes instead of forward slashes in filenames. Not sure if glob-to-regexp supports Windows.

There's a later version forked. kd-glob-to-regexp 0.4.2

@reefhound
Copy link
Author

Looking at index.js of the dependency glob-to-regexp, it appears to be a single function that parses the input string one character at a time and builds the regex string. In the switch cases I don't see a check for "\" which is how the Windows path separator would look.

@tjx666
Copy link

tjx666 commented Jan 24, 2023

can you provide a screenshot of this extension homepage in vscode. I want to ensure you are using my latest version

@tjx666
Copy link

tjx666 commented Jan 24, 2023

can you provide a screenshot of this extension homepage in vscode? I want to ensure you are using my latest version

@reefhound
Copy link
Author

Capture

@reefhound
Copy link
Author

Pretty sure it's the latest. I can see the newest functions in the extension code, though obfuscated and minified.

function fr(r){return f.filesToExcludes.some(e=>(0,he.default)(e).test(r))}function hr(r){return f.filesToIncludes.some(e=>(0,he.default)(e).test(r))}

@tjx666
Copy link

tjx666 commented Jan 24, 2023

I'm outside visiting my relatives.I will test it on windows tonight

@tjx666
Copy link

tjx666 commented Jan 24, 2023

image

You need write **\\*.qss on windows:

image

@tjx666
Copy link

tjx666 commented Jan 24, 2023

@reefhound try latest version

@reefhound
Copy link
Author

Works now. Great job.

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

No branches or pull requests

2 participants