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

Random CoffeeScript file showing up as Appveyor #563

Open
winstliu opened this issue Mar 20, 2017 · 29 comments
Open

Random CoffeeScript file showing up as Appveyor #563

winstliu opened this issue Mar 20, 2017 · 29 comments
Assignees
Labels
bug Confirmed defect in package logic, deprecation notices, and PRs which fix them.

Comments

@winstliu
Copy link

coffeescript-appveyor

The tab icon when you open it is the Coffeescript brown + the Appveyor icon. Switching languages does not help.

[email protected]

@Alhadis
Copy link
Member

Alhadis commented Mar 20, 2017

Right, kick devtools open. Run this:

_FileIcons.fs.paths

Tell me if:

  1. Any paths are using backwards delimiters
  2. Any entries are keyed with values that aren't Directory or File instances
  3. If more than one key exists for a path (...somehow)

Next:

const fubar = _FileIcons.grep("git-repository-spec.coffee");

Show me the contents of:

  • fubar.icons
  • fubar.currentIcon

@Alhadis Alhadis changed the title Random Coffeescript file showing up as Appveyor Random CoffeeScript file showing up as Appveyor Mar 20, 2017
@winstliu
Copy link
Author

The path for C:\Users\user\Documents\GitHub\atom\spec\git-repository-spec.coffee is pointing to uh... C:/Users/user/Documents/GitHub/atom/appveyor.yml...

The two properties are undefined.

@Alhadis
Copy link
Member

Alhadis commented Mar 20, 2017

2017 and Microsoft still insist on using relics of the DOS-era. Christ, I hate those separators...

Right, that's made the catalyst pretty clear. What I can't understand is how it fell through the dozens of normalisePath calls I scattered everywhere to make sure no backwards/weird-Windows paths were used as keys. *sigh*

Thankfully, stuff like this is about to become easier to debug. Almost finished decoupling the filesystem API... -_-

Guess there's nothing insightful in _FileIcons.log?

@winstliu
Copy link
Author

winstliu commented Mar 20, 2017

Oh, all my keys were Windows-style. Their corresponding File was mapped to a Unix-style path though. Is that worrisome?

image

EDIT: As for _FileIcons.log, there's ~400 entries in there. Sorting through them might take a while to find the correct one.

EDIT2: Found it. The path again is appveyor.yml instead of git-repository-spec.coffee.

@Alhadis
Copy link
Member

Alhadis commented Mar 21, 2017

Quite the opposite. To make my life simpler and less prickly, I force Windows paths to use POSIX path separators. Every path is meant to be normalised this way before lookup. Obviously something slipped through the gaps somewhere, because those keys aren't meant to be delimited like that.

Is your process.platform global set to "win32"...?

@winstliu
Copy link
Author

process.platform
"win32"

@Alhadis
Copy link
Member

Alhadis commented Mar 21, 2017

The separator discrepancies are the reason for this schizophrenic mess. If I can plug that, this issue will likely be resolved. Needless to say, this is probably connected to the original issue you filed.

I should also check the latest beta to see if there's anything broken I need to fix... -_-

@Alhadis Alhadis self-assigned this Mar 21, 2017
@Alhadis Alhadis added the bug Confirmed defect in package logic, deprecation notices, and PRs which fix them. label Mar 21, 2017
@Alhadis
Copy link
Member

Alhadis commented Mar 21, 2017

BTW, improved path-handling will need to wait for another release. I didn't wanna hold off publishing v2.0.0, but I didn't want to rush anything either.

Which overlooks the fact I just spent several hours mud-wrestling both NPM and APM to get these versions published, but yeah.

@winstliu
Copy link
Author

Just updated to 2.1.1 and I ❤️ the new test icons though :).

@Alhadis
Copy link
Member

Alhadis commented Mar 21, 2017

Glad to hear that, haha. Spent hours designing 'em. :)

It's interesting how difficult icons are to design well... so little room, so many details can get lost so easily...

@winstliu
Copy link
Author

winstliu commented Sep 23, 2017

@Alhadis is this maybe the same issue that I'm seeing right now?
coffee-disguised-as-folder

This is on the freshly-cloned language-csharp repo.

@Alhadis
Copy link
Member

Alhadis commented Sep 23, 2017

Freshly-cloned? As in, you didn't create new directories or rename files in the tree-view after seeing this?

Have you cloned this repo to its current directory before...?

@winstliu
Copy link
Author

As in, you didn't create new directories or rename files in the tree-view after seeing this?

Nope.

Have you cloned this repo to its current directory before...?

Maybe a long time ago. I have not recently cloned this repo.

Here's the full tree: you can see it happening twice.
csharp-full-tree

@winstliu
Copy link
Author

Introducing: I have no idea what is going on. This is from a new Atom window (atom . -n).
csharp-new-window

@Alhadis
Copy link
Member

Alhadis commented Sep 23, 2017

Could you show the output of apm ls -bi, please? I'm gonna have a look at this from a virtualised Windows environment, since I've been unable to reproduce this on Darwin.

@Alhadis
Copy link
Member

Alhadis commented Sep 23, 2017

When you run AtomFS.paths in the dev-console, and expand the entries list... are the path separators uniform across all entries? Or do some still have backslashes as separators? They should all be forwards-facing...

@winstliu
Copy link
Author

Oh hey, look at this. Want me to file a new issue? Only happened in Window 1 (which is in dev mode).

Uncaught (in promise) TypeError: entity.addEditor is not a function
    at disposables.add.onOpenFile.editor (C:\Users\user\.atom\packages\file-icons\lib\ui.js:39:12)
    at Function.module.exports.Emitter.simpleDispatch (C:\Users\user\Documents\GitHub\atom\node_modules\event-kit\lib\emitter.js:25:14)
    at Emitter.module.exports.Emitter.emit (C:\Users\user\Documents\GitHub\atom\node_modules\event-kit\lib\emitter.js:141:28)
    at disposables.add.atom.workspace.observeTextEditors.editor (C:\Users\user\.atom\packages\file-icons\lib\ui.js:66:19)
    at file:///C:/Users/user/Documents/GitHub/atom/out/app/src/workspace.js:621:54
    at Function.module.exports.Emitter.simpleDispatch (C:\Users\user\Documents\GitHub\atom\node_modules\event-kit\lib\emitter.js:25:14)
    at Emitter.module.exports.Emitter.emit (C:\Users\user\Documents\GitHub\atom\node_modules\event-kit\lib\emitter.js:141:28)
    at file:///C:/Users/user/Documents/GitHub/atom/out/app/src/workspace.js:502:22
    at file:///C:/Users/user/Documents/GitHub/atom/out/app/src/workspace.js:502:82
    at Function.module.exports.Emitter.simpleDispatch (C:\Users\user\Documents\GitHub\atom\node_modules\event-kit\lib\emitter.js:25:14)
    at Emitter.module.exports.Emitter.emit (C:\Users\user\Documents\GitHub\atom\node_modules\event-kit\lib\emitter.js:141:28)
    at PaneContainer.didAddPaneItem (C:\Users\user\Documents\GitHub\atom\src\pane-container.js:266:18)
    at Pane.addItem (C:\Users\user\Documents\GitHub\atom\src\pane.js:628:42)
    at Pane.activateItem (C:\Users\user\Documents\GitHub\atom\src\pane.js:570:12)
    at Workspace.<anonymous> (file:///C:/Users/user/Documents/GitHub/atom/out/app/src/workspace.js:1025:12)
    at Generator.next (<anonymous>)
    at step (file:///C:/Users/user/Documents/GitHub/atom/out/app/src/workspace.js:1:12)

Have a field day with this.
image

@winstliu
Copy link
Author

I'm also getting errors from the github package, so maybe it's not just file-icons.

@Alhadis
Copy link
Member

Alhadis commented Sep 23, 2017

Are you running a beta version of Atom, or has it been built from master directly?

@winstliu
Copy link
Author

I'm running Atom 1.22.0-dev-f762bf954. Should have included that, sorry.

@Alhadis
Copy link
Member

Alhadis commented Sep 23, 2017

Can you test this in a stable environment, please?

@winstliu
Copy link
Author

Will try to.

@winstliu
Copy link
Author

Ok, as this is happening with literally every new repo I'm cloning, I suspect it may be due to my tree-view Map changes. I will attempt to report back soon.

@Alhadis
Copy link
Member

Alhadis commented Sep 27, 2017

@50Wliu Don't forget to leverage the filesystem API's index (global.AtomFS.paths) in case it helps you with debugging. :)

I appreciate the time you're putting towards investigating this.

@winstliu
Copy link
Author

winstliu commented Nov 9, 2017

I tried checking out some new repos while on Atom 1.22.0 stable with only file-icons and my syntax theme installed. While I can't reproduce the incorrect icons right now, I can still confirm that AtomFS.paths is reporting a mix of Unix and Windows-style path delimiters. Directories use Unix-style while Files use Windows-style.

language-csharp is still broken and some entries in the Map map to undefined.
language-csharp-file-icons-grammars

@Alhadis
Copy link
Member

Alhadis commented Nov 20, 2017

Right, this should be solved with the latest release. Could you confirm?

If not the incorrect icons, then certainly the inconsistent path separators. I've added a speciality class to help deal with that.

@winstliu
Copy link
Author

winstliu commented Dec 9, 2017

Right, this should be solved with the latest release. Could you confirm?

Whoops, missed this. I'll get back to you when I have time.

@winstliu
Copy link
Author

language-csharp-still-broken
There is no language-csharp/grammars entry in AtomFS.paths.

I also see this:
settings-view-incorrect-spec-icon
The file does exist in AtomFS.paths and points to the correct File. Changing languages does nothing.

Atom 1.25.0-dev-3039ac1f4, [email protected].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed defect in package logic, deprecation notices, and PRs which fix them.
Projects
None yet
Development

No branches or pull requests

2 participants