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

Adding default fileicon support to language contributions #118846

Merged
merged 13 commits into from
Jan 3, 2022

Conversation

Mai-Lapyst
Copy link
Contributor

This PR fixes #14662. The changes can be test by using a custom language as in the language-configuration-sample with an custom font that uses the contributes.icons and contributes.iconFonts API.

Example package.json:

{
  "name": "language-configuration-sample",
  "displayName": "Language Configuration Sample",
  "description": "Language Configuration Sample",
  "version": "0.0.1",
  "publisher": "vscode-samples",
  "engines": {
    "vscode": "^1.28.0"
  },
  "categories": [
    "Programming Languages"
  ],
  "enableProposedApi": true,
  "contributes": {
    "icons": [
      {
        "id": "foojs-fileicon",
        "description": "FooJS file icon",
        "default": {
          "fontId": "foojs-icon-font",
          "fontCharacter": "\\E000"
        }
      }
    ],
    "iconFonts": [
      {
        "id": "foojs-icon-font",
        "src": [
          {
            "path": "./foojs.woff",
            "format": "woff"
          }
        ]
      }
    ],
    "languages": [
      {
        "id": "foo-javascript",
        "extensions": [
          ".fjs"
        ],
        "aliases": [
          "fjs",
          "Foo-JavaScript"
        ],
        "icon": "foojs-fileicon",
        "configuration": "./language-configuration.json"
      }
    ]
  }
}

The font is generated like the font from the product-icon-theme-sample.

@ghost
Copy link

ghost commented Mar 12, 2021

CLA assistant check
All CLA requirements met.

@gjsjohnmurray
Copy link
Contributor

Can this support different default fileicons for the different file-extensions that an extension can associate with a language?

@Mai-Lapyst
Copy link
Contributor Author

@gjsjohnmurray no it can't (at least at the moment). its implemented like the proposed way from aeschli in this comment #14662 (comment) which only allows one default icon per language definition

@ghost
Copy link

ghost commented Jul 2, 2021

Can we use SVG files instead of fonts? Dark and light icons? What's the syntax for both?

@piersdeseilligny
Copy link

Is there any update on this PR? @aeschli is there something wrong with it?

@@ -20,6 +20,7 @@ export interface ILanguageExtensionPoint {
aliases?: string[];
mimetypes?: string[];
configuration?: URI;
icon?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

We want to use ThemeIcon as the type to represent icons. You can create an instance on the fly { id: iconName; }
But in the contribution point it's fine that it's just a string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, can do. I only ran into a problem when I changed the icon field on the ILanguageExtensionPoint from string to ThemeIcon because of monaco (monacod.d.ts) not seemingly containing/finding this class.

@relu91
Copy link

relu91 commented Oct 11, 2021

It seems that @Mai-Lapyst got the @aeschli's review point covered in the last commit. Any chance to get this PR landed soon? 😃

@aeschli
Copy link
Contributor

aeschli commented Oct 15, 2021

Thanks @Mai-Lapyst!

I made some changes.

  • use ThemeIcon but dont have it in monaco.d.ts
  • use the Use ${iconName) notation in package.json as we already use that notation at other places where icons are referenced
    "languages": [
      {
        "id": "diff",
        "extensions": [ ".diff" ],
        "configuration": "./languages/diff.language-configuration.json",
        "icon": "$(zap)"
      },
  • by default show the mode icons if the theme has file icons. Themes can override with the new theme property showLanguageModeIcons
  • only show the language icon if none of the variants (light, high contrast) has a language icon. This avoids conflicts when the icon theme uses pngs or fonts with different sizes.

@tiagovtristao
Copy link

Nice! This is something that we are really looking forward to. Anything preventing this from going in? I'm happy to help if required.

@hamirmahal
Copy link
Contributor

After adding this feature, how difficult will it be to give custom icons based on a specific filename? For example, if developers have a file called test, with no file extension, in their workspace, will this change allow me to give that file a special, custom icon?

I'm looking forward to this. Thank you to the people adding this feature.

@aeschli
Copy link
Contributor

aeschli commented Jan 2, 2022

@hamirmahal It all builds on the existing ways on how filenames are associated to languages

  • When an extension defines a language we currently support extensions, filenames, filenamePatterns and firstLine.
  • Additionally users can customize filename to language associations using the setting
 "files.associations": {
    "devcontainer*.json": "jsonc"
  },

@hamirmahal
Copy link
Contributor

@aeschli Ah, okay. Thank you for the response!

After merging this pull request, how will we know in which version of VSCode this feature will be available?

@ghost
Copy link

ghost commented Jan 3, 2022

Next endgame is end of January, if this does get deemed ready for merge

@aeschli
Copy link
Contributor

aeschli commented Jan 3, 2022

I made the following change:
icon no longer points to a ThemeIcon but to light and dark paths that provide the icon (svg, gif, png...).

{
  "contributes": {
    "languages": [
      {
        "id": "latex",
        // ...
        "icon": {
          "light": "./latex.png",
          "dark": "./latex.png"
        }
      }
  ]
}

That simplifies the usage of the new property. It is no longer necessary to also create an icon font and we no longer depend on
contribIcons proposed API (https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.contribIcons.d.ts).

@aeschli
Copy link
Contributor

aeschli commented Jan 3, 2022

The new property is currently marked as proposed API. That means to try it out, an extension must add

"enabledApiProposals": [ "languageIcon" ]

to its package.json and cannot yet be published to the marketplace.
See https://code.visualstudio.com/api/advanced-topics/using-proposed-api from more details.

I'll bring the API to the API call so I can hopefully remove the proposed API check soon.

@aeschli
Copy link
Contributor

aeschli commented Jan 3, 2022

See #140047 for the current proposal and implementation

@aeschli
Copy link
Contributor

aeschli commented Jan 11, 2022

The API is now final, "enabledApiProposals": [ "languageIcon" ] is no longer needed.

@tiagovtristao
Copy link

Thanks for getting this in.

I just tried the following:

"languages": [
      {
        "id": "plz",
        "aliases": [
          "Please",
          "please"
        ],
        "filenames": [
          "BUILD"
        ],
        "extensions": [
          ".plz",
          ".build_defs",
          ".build"
        ],
        "icon": {
          "light": "./plz_128.png",
          "dark": "./plz_128.png"
        },
        "configuration": "./syntax/language-configuration.json"
      }
]

And the icon defined above is correctly linked to all extensions, but not the BUILD filename. I was looking into the code and https://github.com/jesseweed/seti-ui/blob/master/styles/components/icons/mapping.less#L548 seems to be what takes precedence. Is there a way to override this?

@aeschli
Copy link
Contributor

aeschli commented Feb 7, 2022

No, that's on purpose.
Specific icons from the file icon theme always win. The language icon is just a fallback for the case a theme does not have a specific icon.

I would recommend filing an issue against the themes if you think the icon they provide needs to be improved.

With Seti it's a bit special at it is generated by us. What we could do is improve our converter. Please file an issue against microsoft/vscode

@tiagovtristao
Copy link

I see, thanks!

@github-actions github-actions bot locked and limited conversation to collaborators Feb 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[file icons] Adding default fileicon support to language contributions
8 participants