Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fix MacOS folder icon handling. Fixes #12789 #12807

Merged
merged 1 commit into from
Sep 29, 2016

Conversation

petetnt
Copy link
Collaborator

@petetnt petetnt commented Sep 29, 2016

This PR handles an rare edge case where MacOS folder icon breaks down the whole FileTreeView. For repro steps see #12789

Background from superuser.com:

What is it?

It's name is actually Icon\r, with \r being the carriage return 0x0D. If letting the shell autocomplete the path in Terminal, it yields Icon^M, ^M being \r.

Icon^M is a file existing in all directories that have a custom icon in Finder. If you change a directory's icon e.g. in its Get Info dialog by pasting an image into the icon in the upper left corner, the Icon^M file is created.

This PR checks for the existence of an file called Icon\r, and bails out of rendering the said fileNode if it's name matches it. This is relatively safe, because you shouldn't be able to create filenames with such special characters yourself.

There's an side effect of failing to create a file called "Icon" if Icon\r already exists in the same folder, but the behaviour matches MacOS Finder, which also fails (silently).

The PR includes adds Icon\r to the default exclusion list

@ficristo
Copy link
Collaborator

If you change instead ProjectModel._shouldShowName would still work?
Since there is already a mechanism to hide elements from the file tree I think is better to change that.

@petetnt
Copy link
Collaborator Author

petetnt commented Sep 29, 2016

@ficristo Sadly I tried it with

var _exclusionListRegEx = /\.pyc$|^\.git$|^\.gitmodules$|^\.svn$|^\.DS_Store$|^Icon\\r$|^Thumbs\.db$|^\.hg$|^CVS$|^\.hgtags$|^\.idea$|^\.c9revisions$|^\.SyncArchive$|^\.SyncID$|^\.SyncIgnore$|\~$/;

But the regexp doesn't catch it for some reason (tried it with the m flag too). Works just fine on RegExp101: https://regex101.com/r/pLbOJH/1

If you have any ideas on how to get it working, I'd totally be into changing the check to that 👍

@ficristo
Copy link
Collaborator

ficristo commented Sep 29, 2016

With one slash does it work? ^Icon\r

@petetnt
Copy link
Collaborator Author

petetnt commented Sep 29, 2016

Agh, of course the test method escapes the \r by itself. Brb, fixing.

Signed-off-by: petetnt <pete.a.nykanen@gmail.com>
@petetnt petetnt force-pushed the petetnt/fix-macos-folder-icon-handling branch from 21773e5 to 0c3b286 Compare September 29, 2016 19:47
@petetnt
Copy link
Collaborator Author

petetnt commented Sep 29, 2016

@ficristo see 0c3b286

Copy link
Collaborator

@ficristo ficristo left a comment

Choose a reason for hiding this comment

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

If it works, LGTM

@petetnt petetnt merged commit 0d1630c into master Sep 29, 2016
@petetnt petetnt deleted the petetnt/fix-macos-folder-icon-handling branch September 29, 2016 19:51
@hadome
Copy link

hadome commented Oct 14, 2016

Thank's but how I execute this command?

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