473329 - Implement code folding for various programming languages

Status: RESOLVED INVALID

(Skywriter Graveyard :: Editor, enhancement, P4)

Description

[Dion Almaer]

The editor should be able to fold (hide) blocks in JavaScript, HTML, CSS.

Comment 2

[Gordon P. Hemsley [:GPHemsley]]

This is a mass migration from Mozilla Labs :: Bespin to Bespin :: Editor.

Comment 3

[Chris J]

Attachment: lay foundations using jslint

This initial patch lays the foundations for code folding. It tweaks jslint so it returns a functions array including the name, first line and last line of each function. This new functions array replaces the current one from Narcissus.

In addition I've added a few fixes to the parser:
* Error / Warning messages now don't appear on the line you're still typing, because that's annoying
* Error / Warning messages are better written and now give extra detail i.e. the row, character and evidence
* Syntax checks now work for css
* Default parser settings are now syntaxcheck: off and jslint: {}

A forthcoming patch will add the rest of code folding:
* fix the editor so functions can be folded and unfolded.
* fix the "outline" command so it folds all functions on the page 

I also have an idea for user interface. When the function is folded, there should be a descriptive line of text in its place explaining how many closures, variables etc are inside that function (all from jslint).

Comment 4

[Dion Almaer]

This puppy is in tip. Looks great. UI wise we will help here too. I want to see a iconic (>) type thing to show the folding, and if you mouse over the fold it would be nice to popup details and even the impl...

Comment 5

[Chris J]

Attachment: v1 of code folding, with minimal UI

Well it was more complicated than I thought, but here is my first attempt at code folding. It applies to tip.

To activate code folding, click in the gutter next to the function. Click again to unfold. The UI is deliberately basic, I wanted to get feedback on the engine before we focus on how it looks.

The basic trick is to distinguish between 'cursor rows' and 'model rows'. If a js file is 100 lines long of which 20 are folded away, then there will be 80 cursor rows and 100 model rows. Translation functions between the two are in the cursor manager. I use jslint.

Known limitations / issues:
* folding only works for javascript functions more than two lines long
* the UI needs work
* folded functions currently have "...". We could make this a lot more descriptive, e.g. using interesting jslint data such as the number of closures in that function, etc
* the "..." should not be editable
* for some reason the parser doesn't run when the document is first opened any more, you have to trigger a document change event first
* maxCols counts every row, not just the unfolded ones
* I think the vertical scrollbars are wrong now
* the code doesn't work for functions folded within functions that are also folded

You can probably find more bugs, let me know!

Comment 6

[Chris J]

Attachment: v2 of code folding, also with minimal UI

OK so bug fixes galore, this thing is ready to test.

Exciting news: this bug has the side effect of almost eliminating the need for the separate Narcissus parser, which would speed up parsing by a factor of around 7-12x for a 600 line javascript file. Just a few tweaks to codecomplete and syntaxanalysis stuff required to produce this speedup which I will submit later.

Also the "outline" command now has the effect of folding every function, rather than opening up a separate information window. It can be undone by typing "outline u".

Just one bug I haven't nailed yet (vertical scroll bar still has some size issues).

Fixes:
   DONE: fix parser so it runs when the document is first opened, not just after the first document change
   DONE: make outline so it folds everything foldable
   DONE: fix maxCols so it only counts visible rows
   DONE: lastline for anonymous functions is one lower than it should be
   DONE: move cursor if it's inside the function that's about to be folded
   DONE: fix functions folded within functions that are folded
   DONE: stop repeated messages every time the parser runs
   DONE: fold a function then one before it, the one after paints its second line rather than the end line
   DONE: remove the extra code folding information line

Comment 7

[Chris J]

Also I'm looking for ideas on the UI for folding, which is still not implemented (as before you fold/unfold by clicking on the gutter). I don't particularly like the "windows explorer" look with + and i buttons. Also, where should the buttons go?

Comment 8

[Dion Almaer]

Chris J,

Very cool. We have a UI session going and should get you a mockup soon for the folding. One aspect is that we would like to experiment with something like: when you mouse over a folded function, if you hang for a second the code pops up inline. This isn't the same as auto-unfolding.... this is a sep popup for a quick scan.

@Malte, I tried to pull from your repo and now I get the experimental multi instance branch that is causing me havoc.... Branches on hg seem broken and we use separate repos for that kind of thing. How do you feel about having a bespin-malde-multiinstance etc and not using branches?

Cheers,

Dion

Comment 9

[Chris J]

Attachment: final functional patch

Right this should be ready to push, it's based to tip. I fixed the tricky scroll bar bug and I have introduced a new minimal UI. You now get "v" or ">" at the fold points, depending on whether it's already folded or not. Click them to fold / unfold. 

Still looking for fancy artwork etc - Dion could you find some help, since my graphic design skills tail off round the "drawing straight lines" stage. It should be easy for me to swap in some better artwork as a followup bug.

Comment 10

[Chris J]

Attachment: final patch including new UI

OK this now even includes a fancy UI. No known bugs. 
Please could someone test and commit - it's ready for release, it's useful code and it's taking me ages to keep resyncing it to tip. Thanks!

Comment 11

[Dion Almaer]

Hi Chris,

We have been iterating on the code folding UI.... but it is coming!

I integrated your latest changes here... what should I be seeing? Instead of seeing code fold points all I can find that is different is that the left edge of the code is 6px to the right.

I assume I have something wrong.

Also, just so you know... the code also goes right through some hot paths so we will give this a really good code review.

I noticed some things commented out:

+        //ctx.beginPath();
+        //ctx.rect(this.gutterWidth, -this.yoffset, cwidth - this.gutterWidth, 
+        //ctx.closePath();

Are you putting in those optimizations?

+//bespin.parser.EngineResolver.register(new bespin.parser.JavaScript(), ['js'])

This is because we don't use Narcissus anymore right?

Comment 12

[Chris J]

Attachment: screenshot of code folding

So I merged to tip again and took the following screenshot to show how it works. I used Firefox 3.5 Preview and it works fine, but I haven't tested in Safari yet because I run Linux. 

The gutter is expanded by several pixels as you can see, to allow space for the folding symbols. The top function has been folded, the other two have not, and the third function is a sub-function of the second. That should give you a picture of most permutations. The code displayed is actually from parser.js.

Comment 13

[Dion Almaer]

Attachment: Code folding mock

Comment 14

[Dion Almaer]

Just attached the code folding mock that shows a very subtle style. Sorry about the patch mess up. Ben and I will be ready to merge in at your convenience after you check out the recent changes.

Comment 15

[Chris J]

Attachment: rebased to tip, a few UI changes

Hi all, this should now work from revision 1824:5c485cb843a7 since bespin didn't work for me from tip, I got some thunderhead issues (Firefox 3.5 Ubuntu).

I have made UI changes as per Dion's screenshot. The folding area is now part of the gutter, the "up" and "right" triangles and "..." that indicate folds now all work.

I haven't yet got the hover-over UI working yet, I'll log a separate bug for that. Also there are some issues with line markers which mean we've got to fix bug 478524 before a release, I'm working on this right now.

Comment 16

[Chris J]

To clarify, this code is ready to test and commit. There are two follow-up bugs
(478524 for line markers and 499544 for the hover-over UI) which will add extra
enhancements soon.

Comment 17

[Chris J]

Attachment: new code folding patch

Rebased to tip. Enables code folding for javascript.

Comment 18

[Ben Galbraith]

We should try and land this in 0.4.1, but if we can't, hit it in the next release

Comment 19

[Chris J]

Attachment: rebased to tip

This rebases to tip. It implements code folding for javascript using the jslint parser.

Comment 20

[Joe Walker [:jwalker]]

When I apply this to my repo, I don't see any folding.

I'm wondering if we need some special value in the jslint setting - the current default of {} is probably wrong - settings should really be strings.

It's possible that something has changed since you rebased to tip. What version was this created against?

Comment 21

[Kevin Dangoor]

ACETRANSITION

The Skywriter project has merged with Ajax.org's Ace project (the full server part of which is their Cloud9 IDE project). Background on the change is here:

http://mozillalabs.com/skywriter/2011/01/18/mozilla-skywriter-has-been-merged-into-ace/

The bugs in the Skywriter product are not necessarily relevant for Ace and quite a bit of code has changed. For that reason, I'm closing all of these bugs. Problems that you have with Ace should be filed in the Ace issue tracker at GitHub:

https://github.com/ajaxorg/ace/issues