Bug fix in pgfSweave's handling of Sconcordance #32

Merged
merged 2 commits into from Mar 20, 2012

Projects

None yet

2 participants

@aecay

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.

@aecay aecay Fix bookkeeping of output lines.
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 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.
2f90e0d
@cameronbracken

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

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

Do you think you might fix this warning?

@aecay

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

No worries, thanks again for your work!

@aecay

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

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

@yihui yihui referenced this pull request in yihui/knitr Feb 12, 2012
Closed

support concordance #133

@cameronbracken cameronbracken merged commit 02e1264 into cameronbracken:master Mar 20, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment