Skip to content

Add support for eslintrc.json seti-ui icon #123404

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

Merged
merged 6 commits into from
Jul 19, 2021
Merged

Add support for eslintrc.json seti-ui icon #123404

merged 6 commits into from
Jul 19, 2021

Conversation

adaex
Copy link
Contributor

@adaex adaex commented May 9, 2021

This PR follows the steps from CONTRIBUTING.md to pull in updates from seti-ui, adding an already existing icon for eslintrc.js file to eslintrc.json

jesseweed/seti-ui already supports the icon for eslintrc.json file. View line 554.

Screenshot of preview.html:

image

@aeschli
Copy link
Contributor

aeschli commented May 11, 2021

Can you explain how you got to these changes?

I just updated the icon theme with the latest what's in seti-theme: 86bc051

  • have the main branches of https://github.com/jesseweed/seti-ui and https://github.com/microsoft/vscode cloned in the same parent folder
  • in the seti-ui folder, run npm install and npm run prepublishOnly. This will generate updated icons and fonts.
  • in the vscode/extensions/theme-seti folder run npm run update. This will launch the icon theme update script that updates the theme as well as the font based on content in seti-ui.
  • to test the icon theme, look at the icon preview as described above.

I updated CONTRIBUTING.md with these clarifications

You migth have to study and correct the update-icon-theme.js. Not sure why it doesn't create a filename rule for eslintrc.json

@ghost
Copy link

ghost commented May 12, 2021

CLA assistant check
All CLA requirements met.

@adaex
Copy link
Contributor Author

adaex commented May 12, 2021

@aeschli , In fact, this problem puzzled me at first, but now I seem to find the reason why the eslintrc.json file icon is not generated.

Let me repeat that.

  1. In line 52 of the file extensions/json/package.json, we can see that the extensions of jsonc is assigned .jsonc, .eslintrc, .eslintrc.json, etc.

image

  1. In extensions/theme-seti/build/update-icon-theme.js, we can see that preferredDef attempts to assign values through extensions and filenames.

image

For jsonc, the extensions .jsonc does not exist in seti-ui, so the value of preferredDef is assigned to the icon of .eslintrc.

  1. According to the following code, we can find that the file allocation of .eslintrc and .eslintrc.json has been deleted.

image

If we still don't understand, we can check the logic. Follow the screenshot below to add some code.

image

Run npm run update, Here is the output:

node ➜ /workspaces/vscode/extensions/theme-seti (eslint-icon ✗) $ npm run update

> [email protected] update /workspaces/vscode/extensions/theme-seti
> node ./build/update-icon-theme.js

Reading from ../../../seti-ui/styles/_fonts/seti.less
lang: jsonc  preferredDef: _eslint
remove the extension association: eslintrc
remove the extension association: eslintrc.json
written ./icons/vs-seti-icon-theme.json
updated ./cgmanifest.json
Updated to jesseweed/seti-ui@2b078f8 (2021-05-05)

We can verify that jsonc is assigned as the _eslint icon, and finally remove the file icon assignment of eslintrc and eslintrc.json.

My current solution is to ignore this part of the operation if the language has been defined in inheritIconFromLanguage.

image

@aeschli aeschli added this to the July 2021 milestone Jul 19, 2021
@aeschli aeschli added the themes Color theme issues label Jul 19, 2021
@aeschli aeschli merged commit 8d20dc7 into microsoft:main Jul 19, 2021
@aeschli
Copy link
Contributor

aeschli commented Jul 19, 2021

Thanks @adaex !

@aeschli aeschli added feature-request Request for new features or functionality verification-needed Verification of issue is requested labels Jul 19, 2021
@adaex adaex deleted the eslint-icon branch July 25, 2021 18:16
@github-actions github-actions bot locked and limited conversation to collaborators Sep 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality themes Color theme issues verification-needed Verification of issue is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants