proposal: prependLeft(), prependRight(), appendLeft(), appendRight()

[kzc]

Motivation

There is a gap in the magic-string API that insertLeft() and insertRight() cannot accommodate.

Proposal

New methods:

• s.prependLeft( index, content )

• s.appendRight( index, content )

Synonyms for existing methods for naming consistency:

• s.appendLeft( index, content )

  • appendLeft is a synonym for insertLeft

• s.prependRight( index, content )

  • prependRight is a synonym for insertRight

Illustrative example:

var s = new MagicString('0123456789');

s.insertLeft(5, 'A');
s.insertRight(5, 'a');
s.insertRight(5, 'b');
s.insertLeft(5, 'B');
s.insertLeft(5, 'C');
s.insertRight(5, 'c');

// s === '01234ABCcba56789'
console.log(s.toString());

console.log('left  of 5:', s.slice(0, 5));
// left  of 5: 01234ABC

console.log('right of 5:', s.slice(5));
// right of 5: cba56789

//
// Proposed methods
//

s.prependLeft(5, '<');
s.prependLeft(5, '{');
// s === '01234{<ABCcba56789'

s.appendRight(5, '>');
s.appendRight(5, '}');
// s === '01234{<ABCcba>}56789'

s.appendLeft(5, '(');   // appendLeft is a synonym for insertLeft
s.appendLeft(5, '[');   // appendLeft is a synonym for insertLeft
// s === '01234{<ABC([cba>}56789'

s.prependRight(5, ')'); // prependRight is a synonym for insertRight
s.prependRight(5, ']'); // prependRight is a synonym for insertRight
// s === '01234{<ABC([])cba>}56789'

console.log('left  of 5:', s.slice(0, 5));
// left  of 5: '01234{<ABC(['

console.log('right of 5:', s.slice(5));
// right of 5: '])cba>}56789'

[alangpierce]

Agreed that more control over these types of inserts would be nice. The decaffeinate project extensively uses magic-string, and many of the bugs I've worked through have been due to subtleties where code snippets are inserted in the wrong order due to the details of how magic-string works. (Not that magic-string is necessarily at fault; it's a hard problem, especially in situations where you need to do many inserts at the same point.)

Here are the rules I've learned to follow when working on decaffeinate:

• Always use insertLeft. This ensures that if you insert multiple things in order at the same point, they will appear in order instead of in reverse order. This is a good justification for the existence of appendRight. One problem with this is when you try to perform modifications to code, then get that code to move it elsewhere. For example, if you wrap an expression in parens and then call slice to get its code, the insertLeft means that you'll miss the open-paren. I ended up writing a patchAndGetCode method to work around this limitation.
• As much as possible, make it so code patching happens in order. For example, if you want to wrap an expression in parens and transform it, insert the left paren, then do the transform, then insert the right paren.
• Split work up into multiple passes when possible. For example, CoffeeScript code can have a lot of "implicit" parens for function calls, and a number of magic-string-related problems were solved by inserting those parens in a separate CoffeeScript to CoffeeScript step before the main transformation.

I've thought a bit about how the magic-string API might be changed to alleviate these problems. One thought I've had is that you could provide a priority number (or some better name) that determines the absolute order of the insert. Negative means before the index (like insertLeft) and positive means after the index (like insertRight). For example, you might put an open-paren at priority 50 and unary negation at priority 100, and then regardless of insertion order, you'd get (-.

Another nice feature would be a way to programmatically examine the fragments that have been inserted at a particular point, and insert and slice between any of those fragments.

But really, while these features would make more advanced use cases possible, it seems like the type of code that uses them may end up not being very pleasant or robust. One interpretation is that magic-string, and any tool with its strategy, naturally gets pretty fragile when doing complicated enough work, and it may be better at larger scales to use a tool like recast where you work on an AST rather than doing string modifications. The decaffeinate project is relatively stable at this point with the practices I mentioned above, so it's unlikely to have to move away from magic-string or to use more advanced features if they were introduced.

[Rich-Harris]

I agree with the proposal to add those methods. I had a feeling something like this might be necessary, but I wanted to see how far we could get with insertLeft and insertRight before needing to add extra API complexity. It looks like we've reached that point with Bublé and Decaffeinate.

I'm hoping that using these new methods in Bublé would fix #111 – it sounds like maybe the erroneously removed code is a casualty of using insertLeft and insertRight to hack around the limitations described above.

I can see how priority-based system such as @alangpierce describes might in theory be necessary. For now though I think we should stick with (ap|pre)pend(Left|Right) – much simpler to implement, and probably addresses 99% of use cases.