leny / atom-tree-ignore

Use a .atomignore file to hide files & folder in tree view
MIT License
41 stars 13 forks source link

Trailing slashes prevent from hiding a folder #13

Closed PhiLhoSoft closed 8 years ago

PhiLhoSoft commented 8 years ago

Say I have foo/bar. If I ignore foo, foo and bar are hidden. But if I ignore foo/, bar is hidden but not foo... That's because Atom provides paths without trailing slash: /x/y/foo and /x/y/foo/bar. The ignore just applies rules against strings, it won't check existence of a folder or a file. So to ensure it is working, this package should check if /x/y/foo is the path of a folder, and if so, it should add a trailing slash...

leny commented 8 years ago

Thanks for the report. I add this to my tasks.

PhiLhoSoft commented 8 years ago

I have a fix for this issue that also addresses the issue #12 in a way different from your (perhaps a bit more efficiently). Both use li classes to identify the project root(s) and the directories. At least, it is consistent... :smile:

I tried to follow your code style and conventions (Hungarian notation, grouping declarations even if let makes this practice a bit obsolete).

I just attach the modified file here, at least so you take a look. I can do a PR if you prefer / agree on my changes. Or just pick and remix the changes / ideas to your liking, I don't mind.

Note: code changes in onDidActivateInitialPackages are only to ease debugging, you can ignore them. But I am not fond of "assign in conditional expressions" and over-long chains of calls...

tree-ignore.js.txt

leny commented 8 years ago

If you don't mind, I prefer a Pull Request : it's easier to track changes and diffs. Thanks :+1:

PhiLhoSoft commented 8 years ago

Of course. I hope you don't mind the "killing two bird with the same stone" PR. Not sure what are best practices here, but it is simpler to give both changes at once.

leny commented 8 years ago

No problem for me.