Skip to content

Change icon name on Linux for icon theme compatibility - #62650

Merged
Daniel Imms (Tyriar) merged 7 commits into
microsoft:masterfrom
beruic:patch-1
Mar 15, 2019
Merged

Change icon name on Linux for icon theme compatibility#62650
Daniel Imms (Tyriar) merged 7 commits into
microsoft:masterfrom
beruic:patch-1

Conversation

@beruic

@beruic Jimmy Merrild Krag (beruic) commented Nov 6, 2018

Copy link
Copy Markdown

This change is made under the assumption that the '-oss' part is removed for final official distribution builds.

The issue it resolves can be read about here: numixproject/numix-core#2964

Fixes #65750

This change is made under the assumption that the '-oss' part is removed for final official distribution builds.

The issue it resolves can be read about here: numixproject/numix-core#2964
@msftclas

Microsoft Contribution License Agreements (msftclas) commented Nov 6, 2018

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@wmyhen6

wmyhen6 commented Nov 7, 2018

Copy link
Copy Markdown

Unable to open

@beruic

Jimmy Merrild Krag (beruic) commented Nov 7, 2018

Copy link
Copy Markdown
Author

wmyhen6 what is it that you are unable to open?
Is it the issue I refer to?
My repository?
Something else?

@Tyriar

Copy link
Copy Markdown
Contributor

Thanks for the PR but this is not all that would be involved in changing this and it could have knock on effects for some other projects. Seems to me like numix should just add a code icon numixproject/numix-core#2964 (comment)

I opened discussion when I joined the team about changing the command from code to vscode to avoid potential conflicts, but at that point it was already too late. Now we're aligning things with the command name, and there haven't been any conflict issues to my knowledge.

@beruic

Jimmy Merrild Krag (beruic) commented Dec 3, 2018

Copy link
Copy Markdown
Author

Daniel Imms (@Tyriar) I see no reason to rename the command. This PR is only to rename the the icon name in the generated .desktop files.

@Tyriar

Copy link
Copy Markdown
Contributor

The same concerns exist for the name though, and they turned out not to be a problem. Changing the icon alone would cause work for us and other teams and I don't think it's worth it when numix can just use "code".

@Foggalong

Copy link
Copy Markdown

Daniel Imms (@Tyriar) The point has been touched on in other issues but to recap for here, the Icon=code is too generic to be reasonably covered by an icon theme. Other applications use that icon name as a call for a generic icon representing code, editors, etc. If we make an icon representing VS Code specifically called code.svg then it will show up in other applications when a call for the (generic) icon with that name is made. The solution we (and a few other theme creators) have gone with for now is to make code.svg a link to the generic text editor icon, but it's understandably left the Linux users of VS Code a bit miffed.

@Tyriar

Copy link
Copy Markdown
Contributor

Other applications use that icon name as a call for a generic icon representing code, editors, etc.

Josh Fogg (@Foggalong) do you have an example of one of these?

@bilelmoussaoui

Copy link
Copy Markdown

The new freedesktop standards asks dev's to use a RDNN for the desktop file name and the icon name to avoid this kind of issues. Please don't change the icon to yet an other name and follow the specs.

@Foggalong

Copy link
Copy Markdown

Daniel Imms (@Tyriar) The topic of VS Code's name has come up that many times across that many different Numix repos now that I'll be dammed if I can find the one which had the examples in it. Admittedly I'd imagine it's less of a problem now than when VS Code first came out because other devs will now avoid the generic name to avoid conflict with VSC, but there'll always be some legacy applications which won't switch.

To clarify on what Bilal Elmoussaoui (@bilelmoussaoui) said (because the terminology used wasn't clear to me), this is an example for VS Code of the Freedesktop icon name standard. It ensures that even if too installed applications have the same name, icon name, etc there won't be a conflict in files.

@Tyriar

Copy link
Copy Markdown
Contributor

Sounds good, I opened #65750 to track this. It needs a bit more work though, mostly in our product build. I don't have time to look into it just now though.

@Foggalong

Copy link
Copy Markdown

Daniel Imms (@Tyriar) Thank you so much for looking into this further!

@Tyriar

Copy link
Copy Markdown
Contributor

João Moreno (@joaomoreno) I think this is ready to go, let me know if you see any problems with it. There's a corresponding commit to distro in the tyriar/62650-linux-icon branch

@joaomoreno João Moreno (joaomoreno) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you run a build and everything works OK, then this lgtm.

@beruic

Copy link
Copy Markdown
Author

Can anyone determine which is the right solution to the current merge conflict?

@Tyriar

Copy link
Copy Markdown
Contributor

Jimmy Merrild Krag (@beruic) I'll be merging this today, it will finally happen this milestone 😉

@Tyriar
Daniel Imms (Tyriar) merged commit 3207ea3 into microsoft:master Mar 15, 2019
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 27, 2020
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.

Use com.visualstudio.code* for Linux icon

7 participants