onivim / oni

Oni: Modern Modal Editing - powered by Neovim
https://www.onivim.io
MIT License
11.36k stars 301 forks source link

Icon diferences #1851

Open adelarsq opened 6 years ago

adelarsq commented 6 years ago

There is a diference between the repositories for some icons. I did notice with the css icon for example:

https://github.com/onivim/oni/blob/bc86337e50845cfd73a029511db364b2000c824c/extensions/theme-icons-seti/icons/seti-icon-theme.json#L163-L169

On the source repository the icon is using '\E019'.

Is there anything wrong in the generation process?

CrossR commented 6 years ago

I've noticed this for filetypes I would expect to have icons like html, css, yml, json, sh.

How are the icons defined? A lot of them are present in the file linked above, but have a generic icon in Oni.

adelarsq commented 6 years ago

@bryphe how data from seti-icon-theme.json is generated? I can help with this.

bryphe commented 6 years ago

Ah, interesting, thanks @CrossR and @adelarsq for looking at this!

@bryphe how data from seti-icon-theme.json is generated? I can help with this.

That would be great!

I actually grabbed this from VSCode's repo - we're using that icon theme as-is: https://github.com/Microsoft/vscode/tree/master/extensions/theme-seti/icons

It looks like they updated it with this commit, but we haven't picked up the update yet: https://github.com/Microsoft/vscode/commit/399ff707dea82edb0f84dfba248186548f8b13d2#diff-d7e663ee7d187cd918f11e8ac668a41f

So the easiest option is probably just to update from there.

IIRC VSCode uses a tool to generate their extension from: https://github.com/jesseweed/seti-ui It looks like the tool is here: https://github.com/Microsoft/vscode/blob/master/extensions/theme-seti/build/update-icon-theme.js - so alternatively, we could try running that directly.

But it shouldn't need modifications beyond that for - we actually recognize the same format for icon themes as VSCode.

Would be great to have the icons working for html, css, json - now that you pointed that out, I miss it! 👍

CrossR commented 6 years ago

I was looking at the Icons for a PR, and thought I'd check this over.

As far as I can tell, this fails since we never actually pass over a language. This means the final part of Icons.ts fails, due to language always being null.

This can be seen in FileIcon.tsx, where getFileIcon doesn't pass over a language (which is what the fuzzy finder uses).

Similarly, over in Tabs.tsx, the language is never passed over as well.

The end result is that only the following checks are done:

To give some context, these are what we miss by not passing the language: https://github.com/onivim/oni/blob/bc86337e50845cfd73a029511db364b2000c824c/extensions/theme-icons-seti/icons/seti-icon-theme.json#L1159-L1196

EDIT: Whoops, not filetype, I meant language.