Add forward and backward deletion to PostEditor

[bantic]

Changes postEditor#deleteCharAt to be postEditor#deleteFrom(position, direction).

Adds unit tests for the postEditor directly that assert that it changes
the AT as expected.

Some cleanup in the editor as well to remove some duplicated code.

fixes #36

cc @mixonic

[mixonic]

👍

[mixonic]

Hm, if we add a deleteFrom can it just be added on top of a deleteCharAt API? I imagine a generic character deletion API would be useful.

Or should the deleteRange just be used. Could be build both a deleteCharAt and deleteFrom on top of deleteRange? I found having all the logic for a given case flowing through one function was really useful for the linked list, ala insertBefore managing append, prepend, insertAfter etc.

[bantic]

Yeah, I think directional deletion makes something like deleteCharAt feel awkward. If we are deleting, the direction matters. And we want to delete from a cursor position rather than at a character offset (that's why I changed the method name from deleteCharAt to deleteFrom). For instance to backward-delete "from" the beginning of a section (joining with the previous section) is semantically different than deleting the character "at" the 0th position of the section (in that case, at least to my ears, no joining would be implied, just removal of the character).

I could see an argument for a deleteCharAt method that had no implication of joining anything and only removes a character in a marker at a given character offset. I'd consider removeCharAt(marker, offset) to be a better name, though.

I still have a little trouble following the many conditionals of LL's insertBefore — I would be hesitant to combine the forward- and backward- deletion cases into a single method for fear that the logic would be tortuous. They have a lot of symmetric similarity, but only a little duplicated code (when the cursor offset is in the middle of a marker — that would be a good candidate to refactor into a removeCharAt method), and there's a real difference between them in that the nextCursor marker and offset do not need to change for forward-deletion but they do in nearly every case for backward-deletion.

[mixonic]

These all seem like different views of deleting a range to me. Backward should ideally be as simple as subtracting a position from the offsets. As for the unified logic, I don’t think that should be take to mean “use one big function”. Instead I think it means to build for one master format of inputs. The semantic uses are then wrappers that adapt to that master format, and the internals of that format we then know to be pretty safe for all cases. The alternative is that you’re implementing the same deletion logic several times, and likely not covering all opportunities for failure. Ideally the cursor doesn’t play into the data model changes. I’m aware it does right now, in that a currentMarker and offset are returned.

[bantic]

I think when we have a lot of potentially different input formats floating around we're likely missing an abstraction. I've been wanting a Position or CursorPosition container that makes passing around positional information easier. The deleteFrom({marker, offset}) could become deleteFrom(positionInstance) in the future.

Having a position abstraction would open up deleteRange so that we could do things like deleteRange(from=cursorPosition, to=cursorPosition.moveBackOne()) and deleteRange(from=cursorPosition, to=cursorPosition.moveForwardOne()). I would like to tackle that in a future PR, though. There is good test coverage for deleteFrom now that we can lean on until then.

[mixonic]

Can this be makeMobiledoc?

Additionally, those docs do not look like valid syntax.

[bantic]

Maybe change it to build or just make? It is used by Helpers.mobiledoc as Helpers.mobiledoc.makeMD, already slightly redundant. Changing to makeMobiledoc would make the code read Helpers.mobiledoc.makeMobiledoc.

The mobiledoc should be valid — it first builds a post and then converts it to mobiledoc.

[mixonic]

build seems fine to me. I'm just railing against abbreviations.

makeMD({post, section, marker, markup} =>
  post([
    section('P', [
      marker('some text', [markup('B')])
    ])
  })

is not valid JS. I ported it to something that parses:

makeMD(({post, section, marker, markup}) => {
  return post([
    section('P', [
      marker('some text', [markup('B')])
    ])
  ]);
});

[bantic]

I see — fixed. And changed to build