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

Atom 1.24.0 odd behavior #704

Closed
dawnerd opened this issue Feb 14, 2018 · 26 comments
Closed

Atom 1.24.0 odd behavior #704

dawnerd opened this issue Feb 14, 2018 · 26 comments
Labels
upstream-issue Valid issue, wrong package. Stuff that file-icons has no control or responsibility over.

Comments

@dawnerd
Copy link

dawnerd commented Feb 14, 2018

As I was opening files the icons changed back to the default JS icon. They were showing the js test icon. Is that something that's changed recently? Maybe latest atom broke something.

@JosephTLyons
Copy link

Did you just upgrade to Atom 1.24.0? I just did and now all my Elisp file icons are showing up as Lisp file icons.

@dawnerd
Copy link
Author

dawnerd commented Feb 14, 2018

Yep, 1.24.0

@JosephTLyons
Copy link

It seems Atom 1.24.0 broke File Icons.

@dawnerd dawnerd changed the title *.test.js icon reverted back to normal js Atom 1.24.0 odd behavior Feb 14, 2018
@Alhadis
Copy link
Member

Alhadis commented Feb 14, 2018

Nothing changed recently in this package, no. It seems there's been a regression in Atom's code. 😕

Sigh... I'll investigate once I'm back at a computer.

@dawnerd
Copy link
Author

dawnerd commented Feb 14, 2018

Some more investigation:

Disabling and re-enabling the package show the correct icons. As soon as the file is opened the icon changes back to the wrong one.

@markmarkyesyes
Copy link

I am experiencing similar issues with .js files on updating to atom 1.24.0.

Icons show correctly until I open the file, then both *.js and *.spec.js files change to a blue jsx icon.

Upon uninstalling and reinstalling, the icons maintain the erroneous changes described above. previously selected files maintain a jsx icon, and unselected files have appropriate icons.

@djamesjones4
Copy link

djamesjones4 commented Feb 14, 2018

Same issue. When I open .js files the icon changes from orange JS icon to blue JSX icon. When I disable and reenable file-icon the icon remains the same.

Using Atom v1.24.0
file-icons v2.1.13

@Alhadis
Copy link
Member

Alhadis commented Feb 15, 2018

Right, this is happening because atom.textEditors.getGrammarOverride is returning a scope-name when it shouldn't be. It's confusing the package's grammar strategy, which reevaluates the icon-to-file mapping whenever an editor is opened.

Previously, atom.textEditors.getGrammarOverride returned nothing if a file wasn't explicitly assigned a grammar using the grammar selector. Whereas now it's always returning something, even for files whose grammars haven't been overridden by the user.

So, yeah... definitely a regression in Atom's code, but I'm not sure how to fix this without breaking existing functionality. 😞

@Alhadis
Copy link
Member

Alhadis commented Feb 15, 2018

As a temporary workaround, you can disable the grammar strategy in the file-icons package's settings. Unfortunately, that'll mean icons don't update when you override the editor's grammar manually, but so far that's the most reasonable solution.

The package relied on atom.textEditors.getGrammarOverride to report an empty string for anything that wasn't manually overridden, so we're kind of stuck. 😕

@JosephTLyons You might wish to install this. 😉

@JosephTLyons
Copy link

JosephTLyons commented Feb 15, 2018

@Alhadis woah, I've been looking for a package that does this specifically for awhile now! Going to give it a shot. Thanks!

May I make a suggestion? I completely missed this package when searching for "Emacs" or "Elisp," it never showed up in searches. Not sure what your search keywords are, but I'm certain many others have missed it as well.

@colbycolby
Copy link

colbycolby commented Feb 16, 2018

Atom v1.24.0
file-icons v2.1.16

Definitely seems like an Atom bug -- my Atom is auto detecting all my JavaScript files as "JavaScript with JSX" and the file icons change to blue jsx even though there's no jsx.

@Alhadis
Copy link
Member

Alhadis commented Feb 16, 2018

Alright guys, this has been fixed upstream in atom/atom#16754 and will probably land in Atom 1.24.1. Gonna close this as there's really nothing more we can do on our end. In the meantime, you can disable the grammar-strategy (as explained above). Remember to run the file-icons:clear-cache command on affected projects if the icons are still wrong.

May I make a suggestion? I completely missed this package when searching for "Emacs" or "Elisp," it never showed up in searches

That's... really weird, because both Emacs and Elisp are listed as keywords. 😕 Really can't explain that one, sorry. 😞

For those curious about what was going on

The package's config file defines a scope property which is matched against a grammar's scopeName property when a user overrides the grammar of a file. Currently, only one icon can be mapped to a scope (a known limitation I hope to fix in future). So if you were to override a .spec.js file's grammar by assigning it JavaScript, it'd still show the JS icon instead of the "JS Test" icon.

That's what's been happening in Atom 1.24: an API method which previously returned nothing for a file without an overridden grammar was incorrectly reporting an override. So the package was behaving as though you had assigned the JS grammar manually to each file (and if you were to do so, you'd see the same effect).

@Alhadis Alhadis closed this as completed Feb 16, 2018
@JosephTLyons
Copy link

Weird. I didn't do anything (as in I didn't implement the workaround you mentioned) and my file icons are now correct. I did nothing since it was broken by Atom 1.24.0 and all-of-the-sudden, they came back and are working as expected.

@Alhadis
Copy link
Member

Alhadis commented Feb 17, 2018

@JosephTLyons Are you referring to your Elisp files now showing the usual Emacs icon?

If so, that'll be because you installed the language-emacs-lisp package. ;) It takes precedence over language-lisp due to alphabetical order - had the latter been named atom-lisp-support, well, you'd still be seeing the wrong icon.

That was how I knew you didn't have my Elisp package installed 😉 (and partly also why I recommended it).

@JosephTLyons
Copy link

@Alhadis , jeez. You are correct. I disabled language-lisp when I downloaded language-emacs-lisp. I already forgot about this. Sorry for this, brain is fried from classes.

@Alhadis
Copy link
Member

Alhadis commented Feb 17, 2018

fried from classes.

Less Java, more Lisp. 😆

@Alhadis
Copy link
Member

Alhadis commented Feb 21, 2018

Guys, once I finish repairing the package's test-suite, I'm gonna roll a patch release that disables the grammar-strategy automatically for Atom 1.24.0 users (and only that version). I've closed the fourth duplicate report just now, and I shudder to think how many people are getting confused by this who aren't checking GitHub to see what's going on.

Which means that after you download/install the next file-icons release, you can safely re-enable the grammar strategy since the package will re-enable it for you once the next Atom release is shipped. It's already been included in v1.25.0-beta1.

@JosephTLyons
Copy link

@Alhadis , sounds excellent! Thank you for going the extra mile to automate that process for others. Any sort of ballpark ETA for this?

BTW, I read your style guidelines, just for fun, and enjoyed the humor sprinkled throughout.

@Alhadis
Copy link
Member

Alhadis commented Feb 21, 2018

Any sort of ballpark ETA for this?

Well the package specs are about 80-90% repaired. Probably in the next couple of days, I assume. :)

BTW, I read your style guidelines, just for fun,

Haha, thank you. 😉 It was lighthearted because I wrote it while high on energy drinks. Make no mistake though, the underlying message is dead serious.

@Alhadis
Copy link
Member

Alhadis commented Feb 21, 2018

Everybody here should also gloss through them and admire my fucked-up OCD uncanny ability to wrap paragraphs at 80 columns exactly without hyphenating or padding any sentences.

If anybody has noticed the way I line-wrap every fucking Git commit message I write too, this should come as no surprise.

I've spent too many years writing neatly-formatted commits, now it's second-nature to me. I can't tell if I should be proud or embarrassed.

@JosephTLyons
Copy link

JosephTLyons commented Feb 21, 2018

@Alhadis , absolutely. No I could 100% tell from your tone that you meant business about your particular style, but I also enjoyed how it was written.

@JosephTLyons
Copy link

Also, I'm a bit blown away by the perfectly consistent use of 80 columns with no padding. I'm not even sure how that could work for every sentence without doing some sort of additional tweaks on words and such, but somehow, you have seem to have pulled it off. Looks very clean.

@JosephTLyons
Copy link

Maybe I'm not doing something correctly, so disregard this if I missed an instruction somewhere...

Not sure if this matters or not at this point, but I disabled "Change on Grammar Override" and it only changed the icons for the files that were currently open. Opening a new file (with an incorrect file icon displaying) after the "Change on Grammar Override" setting is disabled does nothing for me. Reloading Atom or just closing and opening it also didn't do anything. I know you've written code to handle this bug for the next version of File-Icons, but just wanted to make it known that the suggested solution doesn't work for me, unless I'm changing the incorrect setting.

@Alhadis
Copy link
Member

Alhadis commented Feb 22, 2018

Did you remember to clear your cached paths?

If not, that'll be handled automatically too. I've bumped the internal version counter in the Storage class, which will cause all caches to be flushed upon the next install.

@JosephTLyons
Copy link

Ahhh yes, clearing the cache via File Icons: Clear Cache and closing Atom and reopening it worked. Totally missed that. Thanks.. I somehow knew that the issue was some mistake on my end. :)

@JosephTLyons
Copy link

I downloaded the newest update and reenabled the grammar option and all seems to be working correctly. Files are displaying correctly no matter which way I set the setting. Very nice work and appreciate the fast fix.

@Alhadis Alhadis added bug Confirmed defect in package logic, deprecation notices, and PRs which fix them. upstream-issue Valid issue, wrong package. Stuff that file-icons has no control or responsibility over. and removed bug Confirmed defect in package logic, deprecation notices, and PRs which fix them. labels Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream-issue Valid issue, wrong package. Stuff that file-icons has no control or responsibility over.
Projects
None yet
Development

No branches or pull requests

6 participants