[CLOSED] Addresses #12456

[core-ai-bot]

Issue by thehogfather
Tuesday Jun 14, 2016 at 07:07 GMT
Originally opened as adobe/brackets#12521

---

Addresses issue where code folding triangles in the gutter were sometimes not correctly updated after deleting lines above a folded region.

---

thehogfather included the following code: https://github.com/adobe/brackets/pull/12521/commits

[core-ai-bot]

Comment by abose
Friday Jul 01, 2016 at 04:53 GMT

---

Tagging@swmitra

[core-ai-bot]

Comment by ficristo
Thursday Jul 21, 2016 at 18:55 GMT

---

I've looked a bit at this.
I was able to reproduce the issue (trying to delete a line of code above a folded region) and indeed this PR fixes it.
@thehogfather can you add a test?

I've noticed a small issue: if collapse a folded region and then expand it, the code folding triangles are not updated until you move the cursor.

I think this is a minimal fix to solve the issue, but the right fix seems to me that we should update all the code of the code folding addon of CodeMirror.

[core-ai-bot]

Comment by thehogfather
Thursday Jul 21, 2016 at 21:16 GMT

---

@ficristo I will have a go at adding some more tests.

Could you clarify what you mean by 'update all the code of the code folding addon of CodeMirror'?

[core-ai-bot]

Comment by ficristo
Friday Jul 22, 2016 at 06:42 GMT

---

If I'm not mistaken, the foldcode.js and foldgutter.js are a port of the CodeMirror ones adapted to work in Brackets. I think we should backport all the changes that are in CodeMirror (better in another PR).

[core-ai-bot]

Comment by thehogfather
Friday Jul 22, 2016 at 09:32 GMT

---

Ah yes - better a different PR. There are lots of subtle differences, mainly to do with the ability to persist and restore fold states.

[core-ai-bot]

Comment by thehogfather
Thursday Jul 28, 2016 at 20:40 GMT

---

Changes ready for another review.
Main changes are updates to tests which now include html files as well as js.

[core-ai-bot]

Comment by ficristo
Friday Jul 29, 2016 at 09:55 GMT

---

@thehogfather thank you a lot!
Could you merge / rebase with master? We have updated CodeMirror, so I would like to test with the new version.
Meanwhile I left some comments.

[core-ai-bot]

Comment by ficristo
Friday Jul 29, 2016 at 19:24 GMT

---

If the newlines I mentioned are necessary as the globals to make testing easier, then is fine to leave them.

[core-ai-bot]

Comment by ficristo
Saturday Jul 30, 2016 at 13:25 GMT

---

@thehogfather could you take a look at the test failures?

So now when a tag span on multiple lines you have two code folding triangles. I'm not an UI / UX expert so I'm not sure is actually a good or bad idea.

/cc@MarcelGerber

[core-ai-bot]

Comment by thehogfather
Saturday Jul 30, 2016 at 14:45 GMT

---

@ficristo regarding the folding triangles, I think this is fine. A better design might be to hide the opening tag when the range is collapsed or to only show the fold triangles at the start of the folded/foldable range (so in your example we dont show line 25 as foldable - only line 29). This is however a little tricky - would need a rewrite of the xml-range finder. We are currently using whatever version is from code-mirror. The same workaround is present in code-mirror. I think it is ok as it doesn't reduce functionality nor does it put the editor in an unusable state.

[core-ai-bot]

Comment by redmunds
Wednesday Aug 03, 2016 at 19:31 GMT

---

Another option would be to collapse both tag attributes and tag contents something like this:

[core-ai-bot]

Comment by ficristo
Monday Aug 08, 2016 at 09:06 GMT

---

I still see some test failures on the latest commit.

[core-ai-bot]

Comment by thehogfather
Monday Aug 08, 2016 at 21:04 GMT

---

@ficristo would you kindly point out the failing test if possible? I typically run the extension test as opposed to 'All tests' so I might have missed something. I've just run the tests again and all are passing. Note that the last commits need a full reload of brackets so that the changes in code mirror are reflected in the tests.

[core-ai-bot]

Comment by thehogfather
Monday Aug 08, 2016 at 21:09 GMT

---

@redmunds interesting idea - I'd however question what happens to the fold mark in the gutter when either of the collapsed ranges in the editor is clicked. They should probably be treated as two independent ranges (as displayed in the editor) and marked as such in the gutter with two fold triangles when open.

[core-ai-bot]

Comment by ficristo
Tuesday Aug 09, 2016 at 09:35 GMT

---

Since the behaviour match the CodeMirror one I'm going to take this as is and I'll open a new issue to discuss more the UI / UX.
I've run multiple times the tests and seems there are some issues with the focus of the test window: when I give the focus on it all the tests pass.

@thehogfather can you squash a bit the commits?

[core-ai-bot]

Comment by thehogfather
Wednesday Aug 10, 2016 at 07:47 GMT

---

@ficristo not sure how much more I can squash this. I have tried without much success. Maybe whoever merges on github can use the Squash and merge button? Alternatively I would probably create a new PR with all the changes if that is preferable.

[core-ai-bot]

Comment by ficristo
Wednesday Aug 10, 2016 at 10:42 GMT

---

@thehogfather thank you.
I've merged as is (I didn't want to mess with github)