Set deparse.max.lines before calling dump()

[lionel-]

options(deparse.max.lines = NA)

Because of wch/r-source@201ddbd

[vspinu]

dump has its own control of deparsing options. I don't think that change is intended to affect dump and dput. Only traceback.

[vspinu]

Ok, this has been fixed here and that basically means that versions between 3.2.1 and 3.5.0 are broken in this regard.

Not sure if having a workaround hanging around in ESS is the right approach. I don't think that many people set the deparse option and fixing it on ESS side would mean adding .ess_dump function, thus uglifying inferior-ess-dump-command. Maybe we just keep it as it is?

[lionel-]

I don't think it made it to 3.5

[lionel-]

I think we should fix it. If only so we can add a unit test for it.

Maybe we could look into some mechanism to manage global options from ESS?

[vspinu]

Maybe we can check if this option is set by the user and display a warning or a message?

Maybe we could look into some mechanism to manage global options from ESS?

Like ess-r-options? Basically adding extra configuration layer. Not sure how it helps with this problem.

[mmaechler]

@lionel- is right: It was detected/fixed too late to enter 3.5.0 which was in "freeze" state by then. I will port it to R 3.5.0 patched so it will be in R 3.5.1.
@vspinu is right, that most users will not have set it, or if set to a reasonable value such as 1000, which also would not harm for 99.9% of the cases. As Vitalie mentioned, a clean solution would use .ess_dump() wrapper function: That could set it, call dump() and reset it to original. I don't think other solutions are ok.

Definitely NOT messing with the user's options() behind his back !!! ESS should never behave like M$ Windows to the user ("I know better what you want than yourself, and I will do for you what you may want...").

[lionel-]

Definitely NOT messing with the user's options() behind his back !!!

It would only be temporary while running the command. It'd be restored in an exit hook (or something equivalent).

[vspinu]

that most users will not have set it, or if set to a reasonable value such as 1000, which also would not harm for 99.9% of the cases.

For me this is a good reason not to fix it. In 2 years people will no longer be using broken R versions but we will be stuck with .ess_dump. This part has been broken in R for almost 2 years, but we had only one complain recently, which is not even clear whether if it was due to this issue or something else.

[lionel-]

I was actually victim of this bug and wondered why dumped objects (notably in Rcpp unit tests) were truncated... It might be generally useful to have a with-ess-r-options macro to solve this without an R wrapper, though this could be tricky for async commands (but this is not needed here). I'll do some experiments with this if I have time.

Oh but actually there might be a simpler solution here: we could just shim base::dump() with a dump function in ESSR. It'd be functionally equivalent so no harm done.

[mmaechler]

No, please ... that's what I already hate much about Rstudio: It uses it's own install.packages() and I have seen bugs that only showed because of that. No please don't go there. It looks simple and easy, but is the same M$ patronizing attitude: "We know better what you want than yourself, and we don't even tell you about it...". No , no, no: Don't shadow anything in search() by ESSR!

[lionel-]

I don't know what microsoft has to do with this. I don't love shimming base functions and it should be a last resort, but oftentimes we do know better than the user and they don't want to know about it. Note that we do it frequently on the Emacs side with advices (there too, as a last resort).

But I understand your objection, no dump() shim then.