Not working with advanced-open-file

[jacobmischka]

It looks like with the recent addition of how consumers work, icons are no longer replaced in advanced-open-file's view. It worked out of the box until 2.0.0. Of course I don't necessarily expect you to support other third-party packages, but that one is pretty popular and did seem to work fine until recently.

It looks like a new consumer would need to be created, is that right? I imagine it would be similar to fuzzy-finder, but that consumer seems pretty complicated so I'm a little lost.

If you could point me in the right direction for adding a consumer that would be great, thank you!

[mikeerickson]

+1

[Alhadis]

The consumers folder is actually just a way of monkey-patching Atom's core packages, because their own icon API is too limited to do what we need it to do.

I'm finishing up work on an icon API that'll actually be useful, and give other package authors an opportunity to list icons for elements that use them.

Sorry about this. I knew this would be a loud change, but it had to be done sooner or later.

[jacobmischka]

Ah okay. Once it's finished please let me know so I can fork advanced-open-file. Thanks!

[Alhadis]

Doing all I can, heh.

[Alhadis]

@jacobmischka To ensure this gets fixed ASAFP, I'll be the one to send advanced-open-file a PR. Once done, I'll ping you in it.

[jacobmischka]

Oh great, thanks so much!

[Alhadis]

@jacobmischka Uhm. I'm guessing you're familiar with React? Because I'm not...

export function PathListItemLabel({path}) {
    let icon = fileService.isDirectory(path) ? 'icon-file-directory' : 'icon-file-text';
    let className = classNames('filename', 'icon', icon);

    return (
        <span className={className} data-name={path.fragment}>
            {path.displayAsParent ? '..' : path.fragment}
        </span>
    )
}

The hellzat? 😄

[jacobmischka]

Ha, yeah I am. Basically that just returns the span that holds the filename. So pretty much it'll just output something like:

`<span class="filename icon icon-file-text" data-name="${path.fragment}">${path.fragment}</span>`

Edit: It'll actually output a react element that equates to that, not just the html string. But similar concept.

What do we need to do to apply the file-icons service?

[Alhadis]

Sorry mate, I just finished simplifying the integration process to be a lot less bothersome. I've updated the package readme with instructions. Do they make sense to you?

[jacobmischka]

Yeah I think so. I'll fork advanced-open-file and give it a shot a little later tonight. Thanks so much for your help!

[Alhadis]

Thank me when it works and hundreds of people are less pissed off... =)

[Alhadis]

Anyway, please let me know the second you have troubles. Seriously.

[jacobmischka]

Does the path passed to the callback need to be absolute?

[Alhadis]

If it has to point to an existing resource, yes. Otherwise, it assumes subfolder/file.js is located at /subfolder/file.js.

Is that going to make things difficult?

[jacobmischka]

No, just wondering.

I'm currently a little confused with the way consumers work in atom in general, my consumeElementIcons function doesn't seem to be getting called ever, so addIconToElement doesn't do anything.

[Alhadis]

Is your File-Icons version in-sync with master?

[jacobmischka]

Oh, no. I didn't realize you hadn't released since. I've got it working, thanks!

[Alhadis]

Awesome, thank fuck. Do the icons update okay when you add a hashbang?

[jacobmischka]

Yep!

Just gotta clean up a few things then I'll submit the PR, thanks again!

[Alhadis]

Goddamn relief. If I weren't running on fumes from days of sleep-deprivation, I'd probably cheer out loud, but yeah...

[jacobmischka]

You deserve a rest!

[Alhadis]

That can wait until I'm dead and/or I've patched some other shortcomings (such as icons not appearing in the tree-view if file-icons is deactivated and reactivated in the same session).

[Alhadis]

... except that's not going to work without risking breakage to what already works. Which is traced back to the original need to support Atom's built-in packages with an ugly monkey-patching approach.

FFS. All I can do at this point is hope there aren't too many people repeatedly deactivating and reactivating the package without reloading the window...

[jacobmischka]

Hm yeah, unless you can convince atom to use your service there's not much that you can do it seems.

[Alhadis]

I'll send them a pull request. Not because I expect them to accept it, but because I want them to realise how myopic it was to deny packages DOM access when consuming file-icons. That detail alone is what's led to this shitfight. 😢

[Alhadis]

Hero! Could I get you to fix Nuclide too? That's the issue I've been most frantic about... It's React/Flow as well...

[jacobmischka]

Sure! I'll take a look tonight sometime, probably in a couple hours.

[mikeerickson]

So, what changed between 1.7.25 and 2.0.x to make things break for Nuclide and Seti. If it wasn't "broke" why fix?

[Alhadis]

@mikeerickson There was a lot that we couldn't do. We couldn't change icons dynamically, the startup time was hellish, maintaining the thing was a freaking nightmare, and everything was built upon one hacky layer of CSS selectors wrapped around another. We couldn't match paths case-insensitively, or partial paths, and were severely limited when it came to matching subpatterns.

[Alhadis]

When I started work on V2, it was under the belief that Atom's new icon API would be the answer to our problems. By the time I realised how limited it was, I was already two-thirds of the way to completion. There've been so many turns of development and design choices, the current state of the package resembles nothing to what I'd initially expected it to.

[mikeerickson]

Makes good sense to me. So is "my" issue related or advanced-open-file or seti theme.

[Alhadis]

Both, but I'll send the Seti theme's maintainer a PR to sort this mess out.

[mikeerickson]

So, what you are saying is, this project is like any other development project. Things never seem to go as planed and at crunch time, all design decisions have already been thrown out the window and it is patch city :)

Thanks for your efforts, greatly appreciated. I will stick with 1.7.25 but would be more than happy to help test any beta etc

[Alhadis]

@jacobmischka: Pull request to Atom sent, though I doubt they'll accept it...

@mikeerickson: Once this gets merged and after @vermotr releases a patch, you'll need to open the seti-classic theme's settings and uncheck Custom Icons. It should look like this:

You can find it by opening Atom's settings, then going "Themes", and then clicking the cog next to the UI theme's name: