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

PR: Add more file type icons to File Explorer #4598

Merged
merged 5 commits into from
Jun 16, 2017

Conversation

andfoy
Copy link
Member

@andfoy andfoy commented Jun 13, 2017

Fixes #3825


Icons are based on font-awesome's file-*-o icons and they are selected based on the inferred MIME type of each file:

captura de pantalla de 2017-06-13 00-16-24

@andfoy andfoy added this to the v3.2 milestone Jun 13, 2017
@andfoy andfoy self-assigned this Jun 13, 2017
@andfoy andfoy changed the title PR: Added more file type icons on file explorer PR: Added more file type icons to file explorer Jun 13, 2017
@ccordoba12
Copy link
Member

Great work @andfoy! I just have one question. On systems other than Windows, I'd like to report the icon of the file a symlink points to. Is this covered in your PR?

@dalthviz, please test this PR and see if it works on Windows correctly.

@andfoy
Copy link
Member Author

andfoy commented Jun 13, 2017

@ccordoba12 I'll check if the icons apply to symlinks, if not, then they will be added! :-)

@ccordoba12
Copy link
Member

Thanks a lot @andfoy!

@andfoy
Copy link
Member Author

andfoy commented Jun 13, 2017

@ccordoba12 Symlinks preserve parent's icon only if the name preserves the original extension, should I override this behaviour?

@ccordoba12
Copy link
Member

No, let's leave things as they are for now as see what users say about this.

We're waiting for @dalthviz to see if this works well on Windows.

@dalthviz
Copy link
Member

@ccordoba12 @andfoy as soon as I manage to fix my Windows installation I will test this 👍

@dalthviz
Copy link
Member

@andfoy @ccordoba12 some of the icons changed 👍 :

imagen

but there is an error:

imagen

I think that instead of AudioFileIcon the correct key is SoundFileIcon

@ccordoba12
Copy link
Member

Also, it's not detecting xlsx as Excel files, and probably the same happens for docx and pptx.

@andfoy
Copy link
Member Author

andfoy commented Jun 13, 2017

Actually, it seems that mimetypes modules is not recognizing docx, xlsx and pptx document extensions:
captura de pantalla de 2017-06-13 14-43-42

@ccordoba12
Copy link
Member

Then force the association using the file extension, please.

@ccordoba12
Copy link
Member

@dalthviz, please run a final test for this one, please.

@dalthviz
Copy link
Member

@andfoy @ccordoba12 testing this is not working, but it's just because a missing dot in the dic that has the MS Office extensions (i.e .xlsx, .docx, .pptx). Also in the line 151 the icon assigment should be something like icon = ima.icon(self.OFFICE_FILES[extension])

@ccordoba12
Copy link
Member

@dalthviz, please give it another try (hopefully the last one :-)

@dalthviz
Copy link
Member

@andfoy @ccordoba12 👍

imagen

@ccordoba12 ccordoba12 changed the title PR: Added more file type icons to file explorer PR: Add more file type icons to file explorer Jun 16, 2017
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @andfoy for your work and @dalthviz for reviewing it!

@ccordoba12 ccordoba12 changed the base branch from master to 3.x June 16, 2017 01:15
@ccordoba12 ccordoba12 changed the title PR: Add more file type icons to file explorer PR: Add more file type icons to File Explorer Jun 16, 2017
@ccordoba12 ccordoba12 merged commit 4a8b976 into spyder-ide:3.x Jun 16, 2017
ccordoba12 added a commit that referenced this pull request Jun 16, 2017
@andfoy andfoy deleted the file_explorer_icons branch June 16, 2017 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants