Turning on repl access in interactive mode #25

Merged
merged 5 commits into from Feb 10, 2017

Conversation

Projects
None yet
2 participants
@llaniewski
Contributor

llaniewski commented Jan 25, 2017

I propose to switch off the --no-readline option in interactive mode.
There are two cases to consider in the interactive mode:

  • fully embedded R application, for which all callbacks are overwritten, and this option doesn't make a difference
  • a console application, which sometimes wants to become an R console (like in the rinside_callback0 example). In such case, readline is very useful, as it provides autocomplete.

I don't see any other method to getting the readline ReadConsole callback, because Rstd_ReadConsole and the UsingReadline flag are not exported in the R library.

Turning on GNU Readline in interactive mode
Switched off the `--no-readline` option in interactive mode
@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Jan 25, 2017

Owner

The patch is short and sweet and looks ok. But I still don't see how you can flip between the normal REPL we use programmatically and a normal one. Maybe I have too little imagination.

Could you cook up an example? That would be an interesting use case if it worked. But so far I am not yet seeing how it could...

Owner

eddelbuettel commented Jan 25, 2017

The patch is short and sweet and looks ok. But I still don't see how you can flip between the normal REPL we use programmatically and a normal one. Maybe I have too little imagination.

Could you cook up an example? That would be an interesting use case if it worked. But so far I am not yet seeing how it could...

@llaniewski

This comment has been minimized.

Show comment
Hide comment
@llaniewski

llaniewski Jan 25, 2017

Contributor

My use case is quite big, as I include R as a plugin in a CFD solver. I'll try to write a small example (rinside_callback2?) and commit it here, a bit later this week.

Contributor

llaniewski commented Jan 25, 2017

My use case is quite big, as I include R as a plugin in a CFD solver. I'll try to write a small example (rinside_callback2?) and commit it here, a bit later this week.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Feb 7, 2017

Owner

Thanks for following up. I will definitely give this a spin. Not sure when I'll get to it so please poke if I seem no responsive. But it is now on the TODO list ...

Owner

eddelbuettel commented Feb 7, 2017

Thanks for following up. I will definitely give this a spin. Not sure when I'll get to it so please poke if I seem no responsive. But it is now on the TODO list ...

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Feb 8, 2017

Owner

This looks nifty, but there is a setup issue. It needs -DRINSIDE_CALLBACKS and when I set that, I get reminded that that feature had been off by default for years:

edd@max:~/git/rinside/inst/examples/standard(master)$ ccache g++ -DRINSIDE_CALLBACKS -I/usr/share/R/include -I/usr/local/lib/R/site-library/Rcpp/include -I/usr/local/lib/R/site-library/RInside/include -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g -O3 -Wall -pipe -Wno-unused -pedantic -Wall    rinside_callbacks2_raw.cpp  -Wl,--export-dynamic -fopenmp  -L/usr/lib/R/lib -lR -lpcre -llzma -lbz2 -lz -lrt -ldl -lm  -lblas -llapack  -L/usr/local/lib/R/site-library/RInside/lib -lRInside -Wl,-rpath,/usr/local/lib/R/site-library/RInside/lib -o rinside_callbacks2_raw
/tmp/ccs4cmaS.o: In function `main':
/home/edd/git/rinside/inst/examples/standard/rinside_callbacks2_raw.cpp:162: undefined reference to `RInside::set_callbacks(Callbacks*)'
/home/edd/git/rinside/inst/examples/standard/rinside_callbacks2_raw.cpp:163: undefined reference to `RInside::repl()'
collect2: error: ld returned 1 exit status
edd@max:~/git/rinside/inst/examples/standard(master)$ 

I presume you rebuilt RInside locally with this on?

Owner

eddelbuettel commented Feb 8, 2017

This looks nifty, but there is a setup issue. It needs -DRINSIDE_CALLBACKS and when I set that, I get reminded that that feature had been off by default for years:

edd@max:~/git/rinside/inst/examples/standard(master)$ ccache g++ -DRINSIDE_CALLBACKS -I/usr/share/R/include -I/usr/local/lib/R/site-library/Rcpp/include -I/usr/local/lib/R/site-library/RInside/include -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g -O3 -Wall -pipe -Wno-unused -pedantic -Wall    rinside_callbacks2_raw.cpp  -Wl,--export-dynamic -fopenmp  -L/usr/lib/R/lib -lR -lpcre -llzma -lbz2 -lz -lrt -ldl -lm  -lblas -llapack  -L/usr/local/lib/R/site-library/RInside/lib -lRInside -Wl,-rpath,/usr/local/lib/R/site-library/RInside/lib -o rinside_callbacks2_raw
/tmp/ccs4cmaS.o: In function `main':
/home/edd/git/rinside/inst/examples/standard/rinside_callbacks2_raw.cpp:162: undefined reference to `RInside::set_callbacks(Callbacks*)'
/home/edd/git/rinside/inst/examples/standard/rinside_callbacks2_raw.cpp:163: undefined reference to `RInside::repl()'
collect2: error: ld returned 1 exit status
edd@max:~/git/rinside/inst/examples/standard(master)$ 

I presume you rebuilt RInside locally with this on?

@llaniewski

This comment has been minimized.

Show comment
Hide comment
@llaniewski

llaniewski Feb 8, 2017

Contributor

As with the other callback examples - the RInside package have to be compiled with RINSIDE_CALLBACK uncommented here: https://github.com/eddelbuettel/rinside/blob/master/inst/include/RInsideConfig.h#L24

In fact, the example and the mod need only the repl function - and repl doesn't really depend on the callback mechanism. Maybe it would be a good idea to pull repl out of the #ifdef RINSIDE_CALLBACK in this pull request (I can add it here if you agree).

Contributor

llaniewski commented Feb 8, 2017

As with the other callback examples - the RInside package have to be compiled with RINSIDE_CALLBACK uncommented here: https://github.com/eddelbuettel/rinside/blob/master/inst/include/RInsideConfig.h#L24

In fact, the example and the mod need only the repl function - and repl doesn't really depend on the callback mechanism. Maybe it would be a good idea to pull repl out of the #ifdef RINSIDE_CALLBACK in this pull request (I can add it here if you agree).

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Feb 8, 2017

Owner

Agreed in principle. Callbacks were a more ambitious undertaking which didn't really pan out, or is used. Exporting repl, if it helps here, should be much simpler. I can look into this too.

Owner

eddelbuettel commented Feb 8, 2017

Agreed in principle. Callbacks were a more ambitious undertaking which didn't really pan out, or is used. Exporting repl, if it helps here, should be much simpler. I can look into this too.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Feb 8, 2017

Owner

Ok, I just pushed a new branch with repl exported. Your example then works (provided one comments out the one line referring to Callbacks. It's a good example -- I like it. Being able to interactively set params has advantages.

Owner

eddelbuettel commented Feb 8, 2017

Ok, I just pushed a new branch with repl exported. Your example then works (provided one comments out the one line referring to Callbacks. It's a good example -- I like it. Being able to interactively set params has advantages.

@llaniewski

This comment has been minimized.

Show comment
Hide comment
@llaniewski

llaniewski Feb 8, 2017

Contributor

I merged the repl modification. As the example doesn't use the callbacks at all - I deleted the callback reference altogether and renamed the example to rinside_interactive1

Contributor

llaniewski commented Feb 8, 2017

I merged the repl modification. As the example doesn't use the callbacks at all - I deleted the callback reference altogether and renamed the example to rinside_interactive1

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Feb 8, 2017

Owner

Splendid. Had the same though about callback/non-callback. Will merge this evening unless I get distracted.

Owner

eddelbuettel commented Feb 8, 2017

Splendid. Had the same though about callback/non-callback. Will merge this evening unless I get distracted.

@eddelbuettel eddelbuettel changed the title from Turning on GNU Readline in interactive mode to Turning on repl access in interactive mode Feb 10, 2017

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Feb 10, 2017

Owner

Ok, I found some time to catch up with RInside and made some (trivial, catching up with tighter R CMD check) changes.

Owner

eddelbuettel commented Feb 10, 2017

Ok, I found some time to catch up with RInside and made some (trivial, catching up with tighter R CMD check) changes.

@eddelbuettel eddelbuettel merged commit bd32d4c into eddelbuettel:master Feb 10, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment