atom-archive / text-document

MIT License
57 stars 10 forks source link

Extract mutation layer out of BufferLayer #5

Closed as-cii closed 9 years ago

as-cii commented 9 years ago

In this PR I basically moved splice from BufferLayer to MutationLayer (including its related specs). This new layer now correctly stores a patch when splice is called.

I'd like to point out the evident duplication between BufferLayer and MutationLayer: it seems like some code wants to be extracted there :art:. Since we're still fleshing out the basics, though, I might be speculating a bit over the design and, as a result, I didn't perform any refactoring. However, here's an idea to keep in mind. Basically, we may have a CombinedIterator which stores two (or even many) iterators with a priority, falling back to the next one when the current hasn't any available content. This would make really evident what BufferLayer's and MutationLayer's responsibilities are (that iteration code hides a bit the intent of those layers).

By the way, in order to complete this layer as described in our README specification, we still need to handle document saving. Since there are a few things which I am not sure of, I'll open a separate PR against this branch to implement it.

/cc: @nathansobo @maxbrunsfeld

nathansobo commented 9 years ago

Hey @as-cii. Really sorry about this but @maxbrunsfeld convinced me yesterday evening that we shouldn't separate these layers. The reasoning is that there's no need to cache content that you've mutated over the top of :grimacing:. Sorry for suggesting it but I think we're going to keep BufferLayer dealing with both concerns for now.

as-cii commented 9 years ago

:cry:

Just kidding! :smile: No problem, I'll close this and update the README. Since you already agreed on removing the layer, I'll push the docs change directly on master.

nathansobo commented 9 years ago

Thanks @as-cii