simplifies the creation of "diff generators" outside the library

[archimag]

I exported some symbols and removed a few generic functions (they are not
needed and only complicate things). This is nessesary for me to write own
"diff generator" for wiki pages. You can view my generator in the work here:
http://cliki2.lisper.ru/history/CLiki/3369769658-3488841891,
the source code here: https://github.com/archimag/cliki2/blob/master/src/diff.lisp .

[froydnj]

First off, thanks for working on making this library useful. This code was a one-off thing written for experimental purposes and I'm glad you're using it and pushing to make it better.

I have two main concerns:

• The PRINT-OBJECT changes are convenient, but IIRC, the reason I wrote separate functions was because of the requirements the standard places on PRINT-OBJECT methods. If you'd like to ensure that the standardese is followed, then I am all for accepting your changes there. Otherwise, I think it'd be better to retain separate functions.
• I'm not a fan of exposing the snake internals to client code. Can you think of a better way to expose that functionality to clients? Or can you make a strong case that one really does need to poke at individual snakes to do useful things?

[archimag]

the reason I wrote separate functions was because of the requirements
the standard places on PRINT-OBJECT methods.

Hmm, I can not understand with this point of view why my code as not well as your. But perhaps just need to replace print-object on something like render-diff and render-diff-window?

I'm not a fan of exposing the snake internals to client code.

Indeed, "snake" is not a common term for all existing LCS algorithms. I can offer is this:

(defclass common-sequence ()
  ((original-offset :reader original-offset :initarg :original-offset)
   (modified-offset :reader modified-offset :initarg :modified-offset)
   (length :reader common-sequence-length :initarg :length)))

(defmethod shared-initialize :after ((seq common-sequence) slot-value &key snake &allow-other-keys)
  (when snake
    (setf (slot-value seq 'original-offset) (original-offset snake)
          (slot-value seq 'modified-offset) (modified-offset snake)
          (slot-value seq 'length) (snake-length snake))))

And instead (in compute-lcs):

(dolist (snake lcs)
  (rotatef (original-offset snake) (modified-offset snake))))

use

(loop for snake in lcs
      do (rotatef (original-offset snake) (modified-offset snake))
      collect (make-interner 'common-sequence :snake snake))

Or simple:

(defclass snake (common-sequence)
  ((lcs :accessor lcs :initform nil)))

[archimag]

I've been thinking for some time, and reworked the code. Your comments?

[froydnj]

This is getting complicated enough that it should be split into two commits, one to manage the printing changes, and one to manage the snake changes. Could you do that, please? I think the changes for printing make sense, and will pull those once you've split them out.

As for the snake changes, I think a better way to go would be to make something like GENERATE-DIFF part of the public API, but instead returning the result of CONVERT-LCS-TO-DIFF. That way the snake bits stay private (the particular algorithm for least common susbsequences shouldn't be exposed, IMHO, just the diffing bits). I note that you are already exporting all the diff-related accessors and so forth, so I think that takes care of the needs of diff generators. Do you agree?

[archimag]

I'm too busy now, I'll do this through a few days.

[archimag]

I split my changes on two commit.

generate-diff has the following problem: diff-generator is designed to create classic "by line" diff. But for a wiki engine I need also "by word" diff. I can independently make it on base compute-lcs call. Other variants - to allocate compute-lcs in a separate library so that I could use it directly or try to develop a universal "diff-by-word-generator". In my opinion the easiest way to allow users to use compute-lcs.

[froydnj]

I don't understand why using COMPUTE-LCS directly is any better than using the result of CONVERT-LCS-TO-DIFF and/or why the information in COMPUTE-LCS is better for by-word diffing. (I can see how you might not want to use diff windows for by-word diffing, but the MODIFIED-DIFF-REGION and COMMON-DIFF-REGION are quite different.) AFAICS, they provide exactly the same information, but in different formats.

Anyway, the changes to use COMMON-SEQUENCE don't seem quite right. Just construct COMMON-SEQUENCE objects directly from the snakes in COMPUTE-LCS and leave the snakes alone; don't try to be clever and minimize allocations. That way, COMMON-SEQUENCE is obviously an external interface, rather than some implementation detail.

[archimag]

AFAICS, they provide exactly the same information, but in different formats.

I guess I just missed it. Now I have a lot of different problems, so I it's hard to go into details. I look at it again.

[archimag]

You were quite right, so even easier. I added function # 'compute-rad-diff and exported needed symbols.

[froydnj]

Looks good, thanks very much!