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

Added php engine. #2144

Merged
merged 4 commits into from
Jul 13, 2022
Merged

Added php engine. #2144

merged 4 commits into from
Jul 13, 2022

Conversation

ralmond
Copy link
Contributor

@ralmond ralmond commented Jun 24, 2022

php should be a simple case like perl.
I've attached a simple test Rmd document.

test.php.Rmd.md

@CLAassistant
Copy link

CLAassistant commented Jun 24, 2022

CLA assistant check
All committers have signed the CLA.

@yihui yihui requested a review from cderv June 24, 2022 02:12
Copy link
Collaborator

@cderv cderv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. php command line seems indeed simple enough to add this way and not require a eng_exec wrapping.

@yihui do we need a test in knitr-example ?

I tested on windows, and this works ok too.

@yihui
Copy link
Owner

yihui commented Jun 27, 2022

@yihui do we need a test in knitr-example ?

No.

R/engine.R Outdated
Comment on lines 137 to 138
mysql = '-e', node = '-e', octave = '--eval', perl = '-E', psql = '-c',
python = '-c', ruby = '-e', scala = '-e', sh = '-c', zsh = '-c', NULL
python = '-c', ruby = '-e', scala = '-e', sh = '-c', zsh = '-c', php = '-r',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's order them alphabetically.

R/engine.R Outdated
@@ -874,7 +875,7 @@ local({
for (i in c(
'awk', 'bash', 'coffee', 'gawk', 'groovy', 'haskell', 'lein', 'mysql',
'node', 'octave', 'perl', 'psql', 'Rscript', 'ruby', 'sas',
'scala', 'sed', 'sh', 'stata', 'zsh'
'scala', 'sed', 'sh', 'stata', 'zsh', 'php'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

@yihui yihui merged commit 1edc8b4 into yihui:master Jul 13, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants