search

noseglid / atom-build

:hammer: Build your project directly from the Atom editor
https://atom.io/packages/build
MIT License
248 stars 97 forks source link

Fix #484, #447 #503

Closed sphaerophoria closed 7 years ago

sphaerophoria commented 7 years ago

In TargetManager, getTargets, and by extension fillTargets, has an implicit requirement that pathTarget.targets is not empty. If it is we re-call refreshTargets which would end up with infinite recursion. We have enough information to retrace the steps fillTargets currently takes so we can just set them ourselves.

There are three failing tests with my current implementation, however these three tests were failing before I made any changes.

noseglid commented 7 years ago

I'm sorry for being so slow to reply to this.

What would really help is if we could add a spec, so we can guarantee it doesn't reappear.

Regarding the code itself. I think it would make more sense to pass a bool to fillTargets called refreshOnEmpty that defaults to true. This should be continually passed to getTargets which should refrain from refreshing if it is false. From the refreshTargets function this should be false.

Doing it like this and we'd avoid duplicating the fillTargets code, keeping it DRY.

noseglid commented 7 years ago

Commited this in #520