ftilmann / latexdiff

Compares two latex files and marks up significant differences between them. Releases on www.ctan.org and mirrors
GNU General Public License v3.0
506 stars 72 forks source link

strike out in math environments #263

Closed xlucn closed 2 days ago

xlucn commented 2 years ago

This is an attempt to achieve strike out in math equations. Reference is the answer here.

This does not solve all the cases. It works well on my own documents. But some complicated equations (e.g., integrations) in the math-related test files in testsuite folder are still underlined. The good thing is, they are not getting worse.

Naikless commented 1 year ago

For fractions, the line is invisible though, because it coincides with the fraction line, right?

Would it be worth a thought to change the delete behavior in math mode to cancel altogether like this:

\RequirePackage{cancel}
\newcommand{\stkout}[1]{\ifmmode\cancel{#1}\else\sout{#1}\fi}

Especially for larger "constructs" like nested fractions, I find the result quite appealing.

ftilmann commented 1 year ago

Apologies for taking more than a year to check out this #PR. It is a nice idea and indeed improves the visual appearance in many cases. Unfortunately I encounter several diff files in the testsuite set, which will no longer compile after this change. In particular, the following are reported as failing (use verify --run in testsuite):

island_obs2004-diff.tex delequ-diff.tex pollack-diff.tex move-equation-diff.tex subscript-diff.tex doubledollar-diff.tex eqnarray-diff.tex eqnarray2-diff.tex subscriptm-diff.tex delequ2-diff.tex gennady-diff.tex outerrise-diff.tex delequ3-diff.tex simplefrac-diff.tex master-diff.tex margalit-diff.tex move-equation2-diff.tex anchordemo-diff.tex The error message on the examples that I looked at is:

\strikeout #1->\ifmmode \text 
                              {\sout {\ensuremath {#1}}}\else \sout {#1}\fi 

OK, this one is in principle easily fixed, just add

\RequirePackage{amsmath}

to preamble. Then nearly all the files in testsuite compile. The one exception is units-diff.tex, which has the clear error message:

! Package cleveref Error: cleveref must be loaded after amsmath!.

See the cleveref package documentation for explanation.

While this is easy to fix in manual post-processing it is annoying.

I clearly cannot accept the PR as is. However, it does represent a real improvement. I can see two options

  1. Add the \RequirePackage{amsmath} to the latexdiff preamble.
    Pro: Works for most files, and user will not even need to know about this to benefit from improvement; easy to implement Contra: adds an additional dependency; causes interference with a few packages.
  2. Check whether amsmath is loaded by file, and if so modify the latexdiff preamble accordingly. Pro: no additional dependency or danger of interference. Contra: lack of discoverability (but power math user, who are most likely to benefit will probably have loaded amsmath in any case); a bit more complicated to implement.

Let me know what you think. In the end I think I tend towards 2. I can take care of the implementation, but might take a while again.

ftilmann commented 1 year ago

@daharn Of course equivalent options would exist for your suggestion. With cancel package I would be even more nervous about adding dependency, as it's not as widespread as amsmath, and for the same reason also the lack of discoverability in option 2 would be much more of an issue. Still, option 2 might be interesting

xlucn commented 1 year ago

Don't worry about it at all. I was only thinking about providing a simple workaround "attempt" that I assumed "not getting worse". I am not a expert in tex, so issues expected. Now that it turns out to be actually worse in some cases, feel free to reject this PR and fix in your code base.

As for the two options, that's a little beyond me as a TeX newbie. Take whichever you think brings less problems :)

ftilmann commented 2 days ago

The idea behind this PR has been implemented with commit bba53fd