WIP: Implement block results

[pfitzseb]

This implements block/underline results and only works on Atom master where https://github.com/atom/atom/pull/9930 has been merged.

The first commit just adds another option for Result to specify the Result's type. Since the code isn't very nice that way (all those switches...) I decided to implement them as subclasses to Result: inlineResult, which is the same as the previous Result, and blockResult, which is the new type of results:

Notably, the class based approach is a breaking change.

For example, the code that produced the above images now looks like this:

fade = not @ink.results.removeLines editor, range.start.row, range.end.row, 'block'
r = new @ink.blockResult editor, [range.start.row, range.end.row],

Still think it's superior -- not sure I can really judge that at this time though :)

ToDos:

• [x] API writeup.
• [ ] Better styling, probably.
• [x] Decide which version to use. Result.type determines the rendering.

Open questions:

• [x] Should activate and co stay static methods of Result? Doesn't make all that much sense imo, but not sure. They'll stay static methods -- seems easier that way.
• [x] Should there be a self sufficient Result class that would be exported? --Result is exported right now, but doesn't really work on its own.-- Since we'll just go with the construction argument approach, yes.
• [x] Do we want a constructor method that takes the type of the result as an argument?

Just in case someone wants to try this (possibly only relevant on Windows): If you have a stable instance of Atom opened at the same time you want to use the version built from master, strange stuff happens (i.e. the second instance will be the same version as the first, even though it's a completly different executable).

[MikeInnes]

Exciting! I'll get atom master set up at some point and give this a thorough look over.

[pfitzseb]

Thanks! I'd also appreciate some input on the design decisions, particularly whether we want different result types to be subclasses of a generic Result.

[MikeInnes]

To be honest, I'm not that convinced by the subclassing design, not least because the inline and block result classes are duplicating a lot of code at the moment, but also because I think we can clean things up conceptually.

I think we can clarify things a bit here by saying that the Results class specifically represents evaluation results, and that they should always have the same semantics; in other words, they should be indistinguishable from an API perspective, but just display differently. That's nice to have anyway because it means people can flip between editor and notebook style results easily. With that constraint I think we only need two switches, one to change 'overlay' to 'block' and one to switch the CSS class.

We can start with that and if we want to have, say, method/docs panels that behave differently to results (e.g. not having the same rules about overlapping, marking tokens rather than blocks), we can have a new class for that, and make sure that there are appropriate utility classes for sharing code.

Does that seem reasonable?

[pfitzseb]

It does. The reason I wanted to at least try the subclass based approach was extensability, pretty much (and I wanted to learn a bit about how classes work in cs :D).

So I went with something close to the first design, although I kept the additional Result.type field. I think that's reasonable, just like the additional type parameter for Result.removeLines, as that's a very useful distinction and necessary to at least have the possibility to have block and inline results coexist.

Thanks for the review, and let me know what you think! :)

[MikeInnes]

This is looking really good! Yeah, classes definitely have their uses and it's well worth seeing where they can come in useful (to avoid mistakes like the ones I made early in the design). You might be also be interested in the console branch for some design tweaks, although the diff is pretty messy unfortunately.

I think it's safe to merge this before the underline API is released, right?

[pfitzseb]

Alright, I moved the styling to the stylesheet.

A class based approach is probably superior to this when it comes to extensibility and stuff, just not my class based approach ;) For now this should be good I think.

I think it's safe to merge this before the underline API is released, right?

Since this defaults to inline results and the syntax didn't change, yes, this works fine on current stable Atom.

Also, I'll have a look at the console stuff when I find the time -- it's pretty important to have a nice design there since it'll probably be used by quite a few people who don't like the inline eval approach.

[rgbkrk]

Exciting!

[MikeInnes]

Docs are much appreciated! I want to make a tiny api change though...