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.

Sign up for GitHub

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

Jump to bottom

Associate pink whale icon from Seti theme with all dockercompose documents #131650

Closed
wants to merge 3 commits into from
Closed

Associate pink whale icon from Seti theme with all dockercompose documents #131650

wants to merge 3 commits into from

Conversation

bwateratmsft
Copy link
Contributor

@bwateratmsft bwateratmsft commented Aug 25, 2021 •
edited
Loading

This PR addresses part of microsoft/vscode-docker#2539.

  1. All dockercompose documents will get the pink whale icon that previously only docker-compose.y(a)ml and docker-compose.override.y(a)ml received.
  2. The file associations to the icons are removed since they are superseded by the above, which determines file associations based on Add a compose language #118042.

@alexr00 @karolz-ms FYI

@bwateratmsft
Provide icon for all compose documents
b18d0f2 
PR #118042 added a `dockercompose` language to help scope down from generic YAML.

This updates the `theme-seti` builtin extension to automatically use the pink whale icon for all documents with language mode `dockercompose`. The same is done for `dockerfile` documents but with a blue icon.

This also removes the per-filename mapping because it is no longer needed; the filename patterns in #118042 cover it.
@bwateratmsft
Remove newline it shouldn't be adding
98dbf9c
@bwateratmsft
Remove newline it shouldn't be adding
eddcf4c
@bwateratmsft bwateratmsft mentioned this pull request Aug 25, 2021
Closed
14 tasks
@aeschli
Copy link
Contributor

aeschli commented Aug 26, 2021

vs-seti-icon-theme.json is generated by a script: https://github.com/microsoft/vscode/blob/main/extensions/theme-seti/build/update-icon-theme.js#L354

That script looks file associations coming from seti and replaces them with a language association.

This doesn't work for dockercompose as it uses filenamePatterns which the script is not looking at.
-> we need to update the script to use filenamePatterns as well. The code is here:

vscode/extensions/theme-seti/build/update-icon-theme.js

Line 366 in f00ad14

// replace extensions for languageId

@aeschli aeschli added this to the September 2021 milestone Aug 26, 2021
@bwateratmsft
Copy link
Contributor Author

@aeschli I think I understand. The right fix might be to update dockercompose to have a few filenames entries. Let me test if that indeed has the expected effect.

@bwateratmsft bwateratmsft mentioned this pull request Aug 26, 2021
Closed
@bwateratmsft
Copy link
Contributor Author

Closing in favor of #131721.

@aeschli
Copy link
Contributor

aeschli commented Aug 27, 2021

I can improve the script...

aeschli added a commit that referenced this pull request Aug 31, 2021
@aeschli
Use filenamePatterns in seti update script. Fixes #131650
5541826
@aeschli
Copy link
Contributor

aeschli commented Aug 31, 2021

I updated the script and the seti theme

@bwateratmsft
Copy link
Contributor Author

Thanks @aeschli!

@github-actions github-actions bot locked and limited conversation to collaborators Oct 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@aeschli aeschli

Labels
None yet
Projects
None yet
Milestone
September 2021
Development

Successfully merging this pull request may close these issues.
2 participants
@bwateratmsft @aeschli