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

file name with `$(xx)` displayed incorrectly in folder view #77747

Closed
OpportunityLiu opened this issue Jul 22, 2019 · 10 comments

Comments

@OpportunityLiu
Copy link

commented Jul 22, 2019

Issue Type: Bug

  1. create folder with a file named $(test).txt in it
  2. open the folder in vs code
    图片

VS Code version: Code 1.36.1 (2213894, 2019-07-08T22:59:35.033Z)
OS version: Windows_NT x64 10.0.18362

System Info
Item Value
CPUs Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz (8 x 2592)
GPU Status 2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
native_gpu_memory_buffers: disabled_software
oop_rasterization: disabled_off
protected_video_decode: unavailable_off
rasterization: enabled
skia_deferred_display_list: disabled_off
skia_renderer: disabled_off
surface_synchronization: enabled_on
video_decode: enabled
viz_display_compositor: disabled_off
webgl: enabled
webgl2: enabled
Load (avg) undefined
Memory (System) 7.83GB (0.85GB free)
Process Argv
Screen Reader no
VM 0%
Extensions: none
@isidorn

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2019

Weird. I can reproduce this.
Assigning to next milestone to investigate

@isidorn isidorn added this to the August 2019 milestone Jul 29, 2019

@OpportunityLiu

This comment has been minimized.

Copy link
Author

commented Jul 30, 2019

try filename $(bold), it will be displayed as a B icon.

@isidorn

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2019

The issue is that the octiconLabel expand function strips the $(test) from the file name here.
This happens for all highlightedLabels.
Assigning to @jrieken and @joaomoreno as the authors of the regex.

@isidorn isidorn removed the file-explorer label Aug 8, 2019

@isidorn isidorn assigned joaomoreno and jrieken and unassigned isidorn Aug 8, 2019

@isidorn isidorn removed this from the August 2019 milestone Aug 8, 2019

@jrieken jrieken assigned isidorn and unassigned joaomoreno and jrieken Aug 8, 2019

@jrieken

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

Nothing to be tweak in the regex. @isidorn you must encode those characters or disable octicons-support when using the labels.

@isidorn

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2019

@jrieken thanks.
I have disabled ocitcon support in the explorer.
I do not think anyone was relying on this. I might be wrong though.

@isidorn isidorn added this to the August 2019 milestone Aug 8, 2019

@isidorn isidorn closed this in a02214a Aug 8, 2019

@jrieken

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

I might be wrong though.

I'd say you are right because you don't control the input strings and if my file is called $(zap).txt it shouldn't show as ⚡️.txt

@jrieken

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

ping @bpasero I believe that ResourceLabels should never default to supporting octicons, esp. when it shows uris/paths

@isidorn

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

Yeah fixing it on that layer makes more sense.
@bpasero let me know if you want me to change the ResourceLabel

@bpasero

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

@isidorn this does not seem to originate from ResourceLabel but rather HighlightedLabel which probably originally was only used in quick open where we always supported octicons. Imho I would start there and disable octicons everywhere it does not make sense.

For example, I can see the HighlightedLabel being used in markers view as well, outside of the ResourceLabel. I am not sure how much sense Octicons make sense there either.

@isidorn

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

@bpasero I have changed that the supportsOcticons is by default false.

I went through all the clients. Here are the only clients which I set to support octicons: quick open, suggest widget.

Clients which do not support octicons: explorer, custom trees (as before), outline tree, call hierarchy tree, references tree.

Let me know if you want me to change some clients to support octicons @jrieken

isidorn added a commit that referenced this issue Aug 9, 2019

@mjbvz mjbvz added the verified label Aug 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.