JBlond/php-diff

Is renderer HTML Inline Diff named right?

Closed this issue · 9 comments

After receiving your app, I did some research and came up with:

  • The term 'Inline Diff' should refer to the inline markings. Each renderer could show inline diffs.
  • The renderer, currently named HTML Inline Diff, is actually a HTML Unified diff.
  • The renderer, currently named HTML Unified Diff, is kind of redundant considering the second bullet.

Striving for perfection, I would say we should change the renderer's name, but this concerns me since this will break the code of users.

Can you do a refactoring for v3?

No problem.

What about

  • HTML Unified diff Table
  • HTML Unified diff simple

On hold until PR #79 is merged, because lots of changes in the affected files.

Since the other html renderers generate tables, I think we should just call it HTML/Unified and, for now, drop the one which doesn't generate a table.
When wanting to offer this dropped renderer again, it's still in the repository.

Perhaps even a better idea to include a single renderer in the lib and create a new repository/package just for the other and new renderers which users can include by themselves with composer.
This way the footprint of the lib remains a lot smaller and requires less maintenance, preventing more releases.

Please feedback your opinion about dropping the non table renderer and about separating the lib and renderers.

Version numbers are cheap. However, what repo names do you suggest? I create them then.

It's not really about versioning... I just think splitting the lib and renderers has pro's:

  • Smaller footprint (less code) of the package.
  • No breaking changes of the lib itself when changing a renderer.

Anyway, let us discuss it away from this topic. ;)

On-topic:

Since the other html renderers generate tables, I think we should just call it HTML/Unified and, for now, drop the one which doesn't generate a table.
When wanting to offer this dropped renderer again, it's still in the repository.

Do you agree with dropping the non-table renderer for now?

Do you agree with dropping the non-table renderer for now?

I suggest create a new repo like you wrote. For v3 branch that is fine.

#81 has been merged.