[CLOSED] LiveDevMultiBrowser should properly search parent directories for an index.html

[core-ai-bot]

Issue by dsrw
Tuesday Dec 16, 2014 at 20:42 GMT
Originally opened as adobe/brackets#10192

---

Presently, LiveDevMultiBrowser gets stuck in an infinite loop trying to step into parent directories to find an initial index.html if none is present in the current directory. Without the fix, the included test will loop forever.

---

dsrw included the following code: https://github.com/adobe/brackets/pull/10192/commits

[core-ai-bot]

Comment by dsrw
Tuesday Dec 16, 2014 at 20:55 GMT

---

I hadn't signed the CLA. Fixed.

[core-ai-bot]

Comment by redmunds
Wednesday Dec 17, 2014 at 00:47 GMT

---

@busykai We're going to make 1 more build for 1.1. Is the benefit vs. risk ratio high enough to consider this fix?

[core-ai-bot]

Comment by busykai
Wednesday Dec 17, 2014 at 03:39 GMT

---

@redmunds, this PR may require some minor re-work. how much time do we have before the next build? this may not be as much of pressing issue since starting LiveDev from a css is [probably] not the most common use case. even though the issue is there, of course, and it has to be fixed.

[core-ai-bot]

Comment by dsrw
Wednesday Dec 17, 2014 at 14:39 GMT

---

In the interest of balancing risk/reward before the 1.1 release, I think the best approach would be to copy the getParentFolder helper into LiveDevMultiBrowser, and update this PR to use that.

I'll do another PR against master that moves getParentFolder into FileUtils (along with tests/docs), and updates both LiveDev modes to use it, along with the getFilenameWithoutExtension change mentioned by@peterflynn. This could land in master as soon as the release branch is merged in, and would be part of 1.2.

Thoughts?

[core-ai-bot]

Comment by busykai
Wednesday Dec 17, 2014 at 15:46 GMT

---

I would much rather prefer a complete change in a single PR. This is not as critical to rush it in.@dsrw, are you up to making the change? Please use getParentDirectoryPath when moving the helper it to FileUtils.

[core-ai-bot]

Comment by busykai
Wednesday Dec 17, 2014 at 15:46 GMT

---

Ah, this is against release, please create a new one against master.

[core-ai-bot]

Comment by dsrw
Wednesday Dec 17, 2014 at 16:03 GMT

---

Yeah, I can make the change, but I'm not sure if I'll have docs/tests ready for the FileUtils changes today. As this is a crash bug, I think it would be worth getting it into 1.1, even if it requires later cleanups.

[core-ai-bot]

Comment by peterflynn
Wednesday Dec 17, 2014 at 17:15 GMT

---

@dsrw Because the timing is so sensitive, we'll finish the rest of that cleanup this morning to make sure this lands in time for 1.1. I'll put up a new PR which includes your first commit to make sure you get credit in the commit history :-)

[core-ai-bot]

Comment by dsrw
Wednesday Dec 17, 2014 at 18:13 GMT

---

@peterflynn much appreciated, thank you. I added the proposed cleanups to the PR, but I won't take offence if you'd rather do it yourself. I also fixed a minor error in the FileUtils.getDirectoryPath tests.

Is there anything else I can do to help with this? Should I resubmit a squashed version of the change? Resubmit against master?

[core-ai-bot]

Comment by peterflynn
Wednesday Dec 17, 2014 at 18:50 GMT

---

@dsrw@busykai Let me know what you think of PR #10207

[core-ai-bot]

Comment by busykai
Wednesday Dec 17, 2014 at 19:04 GMT

---

I'm going to close this one in favor #10207.@dsrw, thank you! Please provide your comments to #10207.