Skip to content

#56286 Add a configuration option to show/hide icons in the breadcrumbs view#78879

Merged
jrieken merged 2 commits intomicrosoft:masterfrom
chrismay:56286
Aug 13, 2019
Merged

#56286 Add a configuration option to show/hide icons in the breadcrumbs view#78879
jrieken merged 2 commits intomicrosoft:masterfrom
chrismay:56286

Conversation

@chrismay
Copy link

@chrismay chrismay commented Aug 10, 2019

Issue #56286.

Adds a config option, breadcrumbs.icons, which controls whether or not icons are shown in the breadcrumbs view

@msftclas
Copy link

msftclas commented Aug 10, 2019

CLA assistant check
All CLA requirements met.

Copy link
Member

Choose a reason for hiding this comment

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

also check for showing symbol icons?

Copy link
Author

Choose a reason for hiding this comment

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

👍

@jrieken jrieken added this to the August 2019 milestone Aug 12, 2019
@jrieken
Copy link
Member

jrieken commented Aug 12, 2019

This looks pretty good. I believe the equally checks need to be extended to also compare the symbol icons - that should be testable when using no tabs - workbench.editor.showTabs-setting

@chrismay chrismay marked this pull request as ready for review August 12, 2019 19:19
@chrismay
Copy link
Author

chrismay commented Aug 12, 2019

Added the symbol icon to the equality check - confirmed that when breadcrumb.icons and editor.showTabs are disabled, symbol icons aren't shown.

Copy link
Member

@jrieken jrieken left a comment

Choose a reason for hiding this comment

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

lgtm. thanks

@jrieken jrieken merged commit 73a91c0 into microsoft:master Aug 13, 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.

3 participants