Joining replace steps

[johanneswilm]

Hey, I have a Transform with multiple steps that may have been rebased on top of other Transforms and their steps. Before sending them around, I want to check if I can simplify the Transform by reducing the number of steps. In Particular, I am looking at ReplaceSteps that follow one-another and their from- and to-attributes.

In JavaScript this is no problem, but if one switches to TypeScript, then suddenly I cannot get to these because they are not part of the publically declared list of attributes. So I am wondering: Is there some other way that I could get to the .to and .from of a ReplaceStep while only using the public API?

Also, I wonder why tr.mapping.invert() is not declared a public method. I assume I can also achieve this by going through the inverted version of each step map, but doing it for the entire Transform seems like a practical shortcut.

[marijn]

@johanneswilm

I want to check if I can simplify the Transform by reducing the number of steps

Have you tried the merge method?

@johanneswilm

suddenly I cannot get to these because they are not part of the publically declared list of attributes.

They are not part of the set of properties in Step, period. Sub-classes might have them (replace steps do), but that probably explains what you are seeing.

@johanneswilm

Also, I wonder why tr.mapping.invert() is not declared a public method.

Looks like that was a typo in the doc comment, rather than anything intentional. Fixed in this patch.

[johanneswilm]

@marijn

Have you tried the merge method?

I did not try it before today, and unfortunately it doesn’t cover all my use cases. I have some steps where one partially cancels out what another step did. So say I have one step that does:

1. “Hello friends, how are you?” => "Hello "
2. "Hello " => “Hello friends, how are you?”

With the merging I do, I see whether they is any space between the two steps, and if there isn’t, I create a new ReplaceStep that takes docs before 1 and after 2 and creates one step out of that, which in this case simple will replace “friends” with “friends”.

Does that make sense?

@marijn

They are not part of the set of properties in Step , period. Sub-classes might have them (replace steps do), but that probably explains what you are seeing.

Ok, does that mean that the TS bindings that someone else maintains should make these available only for ReplaceSteps? Or would you prefer if one did not access the to and from attributes of ReplaceSteps?

[marijn]

@johanneswilm

Does that make sense?

Sounds like it should work!

@johanneswilm

Ok, does that mean that the TS bindings that someone else maintains should make these available only for ReplaceStep s?

I think it makes sense to leave step internals to the step implementation. You could use the step’s map to get at this information portably, though (calling forEach on it to get at the actual changed ranges, and only if the step is an instance of ReplaceStep)

[johanneswilm]

@marijn

Sounds like it should work!

It does.

@marijn

You could use the step’s map to get at this information portably, though (calling forEach on it to get at the actual changed ranges, and only if the step is an instance of ReplaceStep)

Ah, right! I had been looking a bit at the map but hadn’t thought of that.

I found a different way now though where I first check if step.merge(nextStep) works. If this is the case and both step and nextStep are instances of ReplaceStep, I merge as described above.