Skip to content

bugfix: ".txt" files #2

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

Merged
merged 1 commit into from
Sep 12, 2017
Merged

Conversation

billiegoose
Copy link
Contributor

For some reason .txt files weren't working. I think it's because this class just wasn't named correctly? Let me know what you think. Great library!

For some reason .txt files weren't working. I think it's because this class just wasn't named correctly? Let me know what you think. Great library!
@Alhadis
Copy link

Alhadis commented Aug 20, 2017

See here:

By default, icon names are suffixed with "-icon". Set noSuffix to true if you need to specify an icon class in its entirety (such as a default Atom icon). This should otherwise rarely be used.

Atom has a number of built-in icon-classes which the file-icons package also uses (although only a few icons are used this way). Search for noSuffix in the package's main config file for similar examples.

@billiegoose
Copy link
Contributor Author

@Alhadis ok... does that impact the pull request? What should I change?

@Alhadis
Copy link

Alhadis commented Aug 20, 2017

No change needed. It was just FYI.

@websemantics
Copy link
Owner

Thanks @wmhilton for your PR,

A similar issue with other file types confirms @Alhadis's feedback, many thanks for that!

I'll check if it's better to import Atom's built-in icons or renaming to existing ones are the way to go.

@Alhadis
Copy link

Alhadis commented Aug 23, 2017

FYI, I intend to restructure a lot of the code you're currently using. File-icons v2.2 will bring some hefty changes to its infrastructure.

Just a heads up.

@billiegoose
Copy link
Contributor Author

Awesome stuff. I really appreciate being able to use Atom's file-icons in the browser. Thanks guys!

@tcosentino
Copy link
Contributor

FYI @wmhilton I added a PR with these changes and a fix for pdf #3

Thank you for this!

@websemantics websemantics merged commit 6418c44 into websemantics:master Sep 12, 2017
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.

4 participants