variableDiff misses documentation

[LFSaw]

changes introduced in #26 are lacking documentation.

[telephon]

It would be good indeed to have a file with some more tests and examples. Especially the different behaviors of the VarDiff variants should be made explicit and also tested. @rkuivila could you do that?

I'd say that the whole interface is still provisional, once it is safe, I'd like to refactor it into Steno, and at the same time make VarDiff more general so it can be moved to the default library.

[rkuivila]

Hi Julian, Just got back from Austria, so I have a number of things to attend to before I dive back in. But let's talk about the interface and why VarDiff works differently from DiffString: 0. First a bit of philosophy: It seems to me that DiffString is written like a stream filter. I found that everything got easier and more flexible by treating the diff operation as an array manipulation. In particular, it simplifies reordering existing characters. 1. For example, in DiffString, the index "i" shared by all the change functions enforces a coupling between the source and target strings. That prevents matches that alter the order of existing chars. 2. Also, when would removeFunc ever need to know the current length of the target string being constructed? It really acts in relation to the source string. Treating the deletion and insertion positions as the same just makes things awkward. 3. Similarly, "swap" appears to be a deletion and insertion at an index that happens to be within the range of the source string. This seems to reflect the implementation rather than the desired semantics. 4. So, I would propose that delete, insert, retain are the natural categories and swap should be eliminated. RJK On Nov 13, 2017, at 4:26 AM, Julian Rohrhuber <[email protected]<mailto:[email protected]>> wrote: It would be good indeed to have a file with some more tests and examples. Especially the different behaviors of the VarDiff variants should be made explicit and also tested. @rkuivila<https://github.com/rkuivila> could you do that? I'd say that the whole interface is still provisional, once it is safe, I'd like to refactor it into Steno, and at the same time make VarDiff more general so it can be moved to the default library. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#37 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ACHUyF1uPIGkB22SYgKQArRIVFiVVd05ks5s2AtOgaJpZM4QbcM0>.

[telephon]

Well, there is not much philosophy behind DiffString apart from the one that the programmers of the unix diff have cast into it. It is really just an interface to the unix diff, and it makes those distinctions. It has nothing to do with the steno semantics, as long as:

1. the diff semantics are explainable and make sense in the steno paradigm
2. they can be executed withing the steno paradigm

So, I would propose that delete, insert, retain are the natural categories and swap should be eliminated.

Yes, why not. I think it might be better to make these operations part of the external interface of steno, so you can also put a letter somewhere by hand.

My suggestion is that I open a branch where I do a refactoring until VarDiff can operate on steno from without.

[rkuivila]

Hi Julian, OK, sounds good. I will generate some more examples for the current fork. But, thinking about making VarDiff general purpose... It strikes me that there is no "correct" order of execution of update functions. Deletions might be need to be before or after additions depending on what is being represented. So, how to engineer this is not really clear. If we treat char, sourceIndex, and targetIndex as arguments, their values actually identify which function to call: targetIndex == Nil ==> removeFunc sourceIndex == Nil ==> addFunc target and source notNil ==> keepFunc So the user could iterate that array of triples and select functions based on the args. That would give the user the option to reorganize things as needed with a sort. Wait a day, as I may take a quick crack on that overnight. If so, I will push it as an added class (VarDiffString). Cheers, RJK [formatted by telephon for readability]

[telephon]

It strikes me that there is no "correct" order of execution of update functions. Deletions might be need to be before or after additions depending on what is being represented.

So the user could iterate that array of triples and select functions based on the args. That would give the user the option to reorganize things as needed with a sort.

Yes, that's true for the general case. But will indices stay consistent with order? When removing an item you'll have to keep track of the shift of indices that come after it.

Similarly, "swap" appears to be a deletion and insertion at an index that happens to be within the range of the source string. This seems to reflect the implementation rather than the desired semantics.

Actually, thinking about it again, I'd say the opposite: reducing swap to remove + insert is exposing an operational implementation detail. Swapping one element against another has a specific meaning, that in some cases you might want to account for (e.g. one might want to swap a synth, but keep the parameters). But for the current case it doesn't matter and I don't mind if we don't want to support it.

[LFSaw]

reducing swap to remove + insert is exposing an operational implementation detail. Swapping one element against another has a specific meaning, that in some cases you might want to account for

+1