search

cameronbracken / pgfSweave

Quality graphics and speedy compilation with Sweave
http://r-forge.r-project.org/projects/pgfsweave/
29 stars 6 forks source link

Bug fix in pgfSweave's handling of Sconcordance #32

Closed aecay closed 12 years ago

aecay commented 13 years ago

Sweave keeps track of what lines in the weaved output correspond to what lines in the source. This is useful for programs like the patchDVI R library, which allow one to click on a spot in the compiled PDF file and be taken to the line in the Rnw file which generated that portion of the PDF.

Previously, pgfSweave was mangling this info. This commit introduces a helper function through which all code-chunk output should be passed. As a side effect, it moves a lot of bolierplate into the helper function, cleaning up the body of the pgfSweaveRunCode function.

cameronbracken commented 13 years ago

This is great! A much needed fix, I never use concordance so the suport for it was very haphazard, this is also a nice cleanup of the code. See my comment on the commit.

cameronbracken commented 13 years ago

I think there is still one small bug when the tidy option is used with the keep.source option and a long code line or comment is wrapped. After running patchSynctex I get the warning:

Warning message:
In matrix(values[-1], nrow = 2) :
  data length [33] is not a sub-multiple or multiple of the number of rows [2]

see: https://gist.github.com/1036173 for an example.

It doesn't seem to negatively affect the output but it is annoying.

cameronbracken commented 13 years ago

Do you think you might fix this warning?

aecay commented 13 years ago

Sorry for the delayed response -- I've been on vacation and haven't had a chance to look in detail at what is going on in this case. The concordance is supposed to contain an odd number of numbers; superficially it looks like this case is causing an even number to be included. But whether this is a problem with pgfSweave, with patchDVI, or something inherited from base Sweave is not something I've been able to look at.

I'll be back from vacation on the 4th, so I hope to have a bugfix out within a few days of that time.

cameronbracken commented 13 years ago

No worries, thanks again for your work!

aecay commented 12 years ago

OK, I've had a chance to look more at this problem, and figure out what is causing it. The warning message above is caused by the fact that the concordance (which is supposed to be a list of numbers) winds up containing NAs. This is what causes the parsing code in patchSynctex to barf.

It seems the NAs are caused by the keep.source option -- setting that to FALSE gives OK results for highlight=T. As far as I can see, this option only affects the behavior of the deparse(.tidy) call in pgfSweaveRuncode. I still haven't figured out how this percolates through the rest of the function to make NAs crop up. I'll keep trying to understand what goes on and update again when I have something more.

cameronbracken commented 12 years ago

Cool, good to know, let me know if I can help at all.