Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--filter-script option added #167

Merged
merged 2 commits into from
Jun 16, 2019
Merged

Conversation

jasonmccsmith
Copy link

As per Issue 165, --filter-script option added that is applied to both old and new files before diffing.

@ftilmann
Copy link
Owner

Sorry that it has taken me so long to look at this - very busy time at work. This looked good on inspection but I just tested it and it has the following issues:

  1. If filter-script option is not given
    (a) a warning Use of uninitialized value $filterscript in string ne at /home/tilmann/bin/latexdiff line 818. is issued.
    (b) latexdiff does not generate any diff file (this is obvious from the code), filter subroutine returns nothing when test for $filterscript is false or undefined

  2. The output of the filter function seems to be additionally printed out, which is undesirable as it removes command line history and obscures messages , e.g.
    latexdiff --filter-script=cat test-old.tex test-new.tex> diff.tex
    works as expected just as latexdiff without filter, except that additionally the contributing files are printed out.

  3. The status information on number of characters passed to the filter script and received from it should only be printed if verbose flag is set (add if $verbose) behind corresponding print statements.

  4. The usage information for filter-script option needs to be added (Note that there is the brief help shown with -h flag following line 3522 , and the possibly more detailed help in the docstring following l. 3771. The filter-string option is simple enough that the same text can be used, but the markup is a little different.

1,3 and 4 are relatively trivial to deal with, but 2 I am not immediately able to do myself, as I am unfamiliar with IPC, so it would be great if you could update the PR in the light of these comments

@jasonmccsmith
Copy link
Author

jasonmccsmith commented Mar 30, 2019 via email

@jasonmccsmith
Copy link
Author

jasonmccsmith commented May 26, 2019 via email

@jasonmccsmith
Copy link
Author

jasonmccsmith commented May 26, 2019 via email

@ftilmann
Copy link
Owner

I was unable to see what you were describing with #2, however...
latexdiff base.tex rev.tex --filter-script=cat > diff.tex
latexdiff base.tex rev.tex --filter-script=which cat > diff.tex
Both of these result in no output to the console, and no duplicated lines in the diff.tex file.

For these commands I do see the output in the console, which must be on STDERR, as STDOUT is redirected to diff.tex (i.e. no duplicate lines in diff.tex)
Not sure why we are seeing different behaviour, possibly a change of behaviour of IPC depending on version number. The man page gives (on Ubuntu 16.04 LTS version)
perl v5.22.1 2018-11-19
Probably it's difficult and not worth tracking that down, especially I don't know how you would be able to replicate this. Your proposed work-around sounds a little dangerous: if there is a true error message, it would be good to be shown this. Still, for now I guess it is all that can be done.
Have you actually updated the pull request on github yet? I cannot see it on github, also not on jasoncsmith/latexdiff (might be my fault for not looking in the right place, of course).

@jasonmccsmith
Copy link
Author

jasonmccsmith commented May 26, 2019 via email

…ts. Added --ignore-filter-stderr option to bury STDERR from the filter script but only on demand.
@ftilmann ftilmann merged commit 7a41bd7 into ftilmann:master Jun 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants