Execute Script window statement by statement rather than line by line

[lloyddewit]

Replaces PR #8494.

Fixes #7014.
Fixes partially #7989 (allow = to be used as an assignment operator).
Fixes partially #8533 (show R code comments in output window, new pipe operator and multi-line statement; plus third block of code).
This change will make 'ctrl-enter' execute the R statement at the cursor (rather than just the line), this should fix the current bugs with multi-line statements.

Ongoing, not yet ready for test/review.

[lloyddewit]

TODO (please ignore, note for self):

• The code in issue A small problem loading a file into the script window #8509 does not seem to run correctly. PR Fixed double imports of data when using script window #8539 intends to fix this issue but this does not seem to work in this PR. Investigate.
• If RScript cannot parse the R code, then pass R code as string directly to R environment
• Test the code in issue More sample scripts and the third has piping which may work now? #8533
• Move caret before executing statement (looks more natural to user)
• Move caret to next non-blank line, treat white-space lines as blank
• If statement taking longer than expected to execute, then show warning <-- moved to In Script window, when executing statement-by-statement, add message when statement slow to finish #8519
• extra spaces could theoretically mean that wrong statement is executed, so sanity check statement before executing <-- moved to issue In Script window, ensure that statement at cursor is always correctly identified #8569
• Issue Interesting code for layout and possible improvements in the script window #8500 contains ,], this cannot currently be parsed by RScript <-- moved to [, and ,]trigger an exception lloyddewit/RScript#18 for this
• If the statement below (from issue A small problem loading a file into the script window #8509) is at end of script and it has no newline, then comment is appended to output: <-- moved to Comment incorrectly added to suffix lloyddewit/RScript#20

If statement has newline, then output is as expected.

[lloyddewit]

@rdstern
This is not yet complete but you are welcome to start testing if you wish.
Thanks

[rdstern]

@lloyddewit many thanks - will do

[rdstern]

@lloyddewit I was initially confused about what runs easily and what is strict? I think I have now found the reasons, see below, why one script runs fine and a seemingly similar one does not.

I found 2 reasons anyway, and wonder if they might be possible (even easy?) to fix?!

This first one runs fine with the cursor at the start of each line and pressing run. Then the second starts - to my mind - identically, but gives me this:

But that is when I try to run the first line, that just gives the library command?

I think there must be some checking of the whole code, before anything is run? Then something in some codes affects all the imterpretation. Because when I delete the last half - so after the command vc(lmn), (so after about line 50) then the initial commands seem to run ok?

Aha I note I deleted the section that starts with if(0){ ! I am now going to comment out that line!

I think I have the problem(s) isolated. There are 2 in this second code:
a) I did need to comment out the if(0){ together with the closing bracket.
b) I also had to comment out lines with a square bracket. for example: dat <- dat[order(dat$tree,dat$day),]

library(agridat)
data(hanover.whitepine)
dat <- hanover.whitepine

libs(lattice)
# Relatively high male-female interaction in growth comared
# to additive gene action. Response is more consistent within
# male progeny than female progeny.
# with(dat, interaction.plot(female, male, length))
# with(dat, interaction.plot(male, female, length))
bwplot(length ~ male|female, data=dat,
       main="hanover.whitepine - length for male:female crosses",
       xlab="Male parent", ylab="Epicotyl length")

# Progeny sums match Becker p 83
sum(dat$length) # 380.58
aggregate(length ~  female + male, data=dat, FUN=sum)

# Sum of squares matches Becker p 85
m1 <- aov(length ~ rep + male + female + male:female, data=dat)
anova(m1)

# Variance components match Becker p. 85
libs(lme4)
libs(lucid)
m2 <- lmer(length ~ (1|rep) + (1|male) + (1|female) + (1|male:female), data=dat)
#as.data.frame(lme4::VarCorr(m2))
vc(m2)
##         grp        var1 var2    vcov  sdcor
## male:female (Intercept) <NA> 0.1369  0.3699
##      female (Intercept) <NA> 0.02094 0.1447
##        male (Intercept) <NA> 0.1204  0.3469
##         rep (Intercept) <NA> 0.01453 0.1205
##    Residual        <NA> <NA> 0.2004  0.4477

# Becker used this value for variability between individuals, within plot
s2w <- 1.109

# Calculating heritability for individual trees
s2m <- .120
s2f <- .0209
s2mf <- .137
vp <- s2m + s2f + s2mf + s2w  # variability of phenotypes = 1.3869
4*s2m / vp # heritability male 0.346
4*s2f / vp # heritability female 0.06
2*(s2m+s2f)/vp # heritability male+female .203
# As shown in the boxplot, heritability is stronger through the
# males than through the females.

And this is the second script: I must be missing something. Is there something in the whole script that is being checked and affects how anything is processed in that script?

  library(agridat)
  data(harris.wateruse)
  dat <- harris.wateruse

  # Compare to Schabenberger & Pierce, fig 7.23
  libs(latticeExtra)
  useOuterStrips(xyplot(water ~ day|species*age,dat, as.table=TRUE,
                        group=tree, type=c('p','smooth'),
                        main="harris.wateruse 2 species, 2 ages (10 trees each)"))


  # Note that measurements on day 268 are all below the trend line and
  # thus considered outliers.  Delete them.
  dat <- subset(dat, day!=268)


  # Schabenberger figure 7.24
  xyplot(water ~ day|tree,dat, subset=age=="A2" & species=="S2",
         as.table=TRUE, type=c('p','smooth'),
         ylab="Water use profiles of individual trees",
         main="harris.wateruse (Age 2, Species 2)")
  
  # Rescale day for nicer output, and convergence issues, add quadratic term
  dat <- transform(dat, ti=day/100)
  dat <- transform(dat, ti2=ti*ti)

  # Start with a subgroup: age 2, species 2
  d22 <- droplevels(subset(dat, age=="A2" & species=="S2"))

  # ----- Model 1, for subgroup A2,S2

  # First, a fixed quadratic that is common to all trees, plus
  # a random quadratic deviation for each tree.
  
  ## Schabenberger, Output 7.26
  ## proc mixed;
  ##   class tree;
  ##   model water = ti ti*ti / s;
  ##   random intercept ti ti*ti/subject=tree;

  libs(nlme,lucid)
  ## We use pdDiag() to get uncorrelated random effects
  m1n <- nlme::lme(water ~ 1 + ti + ti2, data=d22, na.action=na.omit,
             random = list(tree=nlme::pdDiag(~1+ti+ti2)))
  vc(m1n)
  ##       effect variance    stddev
  ##  (Intercept)   0.2691 0.5188
  ##           ti   0      0.0000144
  ##          ti2   0      0.0000039
  ##     Residual   0.1472 0.3837

  if(0){  
    # Various other models with lme4 & asreml
    
    libs(lme4, lucid)
    m1l <- lmer(water ~ 1 + ti + ti2 + (1|tree) +
                  (0+ti|tree) + (0+ti2|tree), data=d22)
    vc(m1l)
    ##      grp        var1 var2   vcov  sdcor
    ##     tree (Intercept) <NA> 0.2691 0.5188
    ##   tree.1          ti <NA> 0      0
    ##   tree.2         ti2 <NA> 0      0
    ## Residual        <NA> <NA> 0.1472 0.3837


    # Once the overall quadratic trend has been removed, there is not
    # too much evidence for consecutive observations being correlated
    ## d22r <- subset(d22, !is.na(water))
    ## d22r$res <- resid(m1n)
    ## xyplot(res ~ day|tree,d22r,
    ##        as.table=TRUE, type=c('p','smooth'),
    ##        ylab="residual",
    ##        main="harris.wateruse - Residuals of individual trees")
    ## op <- par(mfrow=c(4,3))
    ## tapply(d22r$res, d22r$tree, acf)
    ## par(op)
    
    # ----- Model 2, add correlation of consecutive measurements
    
    ## Schabenberger (page 516) adds correlation.
    ## Note how the fixed quadratic model is on the "ti = day/100" scale
    ## and the correlated observations are on the "day" scale.  The
    ## only impact this has on the fitted model is to increase the
    ## correlation parameter by a factor of 100, which was likely
    ## done to get better convergence.
    
    ## proc mixed data=age2sp2;
    ##   class tree;
    ##   model water = ti ti*ti / s ;
    ##   random intercept /subject=tree s;
    ##   repeated /subject=tree type=sp(exp)(day);

    ## Same as SAS, use ti for quadratic, day for correlation
    m2l <- lme(water ~ 1 + ti + ti2, data=d22,
               random = ~ 1|tree,
               cor = corExp(form=~ day|tree),
               na.action=na.omit)
    m2l # Match output 7.27.  Same fixef, ranef, variances, exp corr

    # vc(m2l)
    ##       effect variance stddev
    ##  (Intercept)   0.2656 0.5154
    ##     Residual   0.1541 0.3926

    # ---





    ## Now use asreml.  When I tried rcov=~tree:exp(ti),
    ## the estimated parameter value was on the 'boundary', i.e. 0.
    ## Changing rcov to the 'day' scale produced a sensible estimate
    ## that matched SAS.
    ## Note: SAS and asreml use different parameterizations for the correlation
    ## SAS uses exp(-d/phi) and asreml uses phi^d.
    ## SAS reports 3.79, asreml reports 0.77, and exp(-1/3.7945) = 0.7683274
    ## Note: normally a quadratic would be included as 'pol(day,2)'

    libs(asreml)
    d22 <- d22[order(d22$tree, d22$day),]
    m2a <- asreml(water ~ 1 + ti + ti2,
                  data=d22,
                  random = ~ tree,
                  residual=~tree:exp(day))
    vc(m2a)
    ##         effect component std.error z.ratio constr
    ##  tree!tree.var    0.2656   0.1301      2      pos
    ##     R!variance    0.1541   0.01611     9.6    pos
    ##      R!day.pow    0.7683   0.04191    18    uncon


    # ----- Model 3. Full model for all species/ages.  Schabenberger p. 518

    ## /* Continuous AR(1) autocorrelations included      */
    ## proc mixed data=wateruse;
    ##   class age species tree;
    ##   model water = age*species age*species*ti age*species*ti*ti / noint s;
    ##   random intercept ti / subject=age*species*tree s;
    ##   repeated / subject=age*species*tree type=sp(exp)(day);
    

    m3l <- lme(water ~ 0 + age:species + age:species:ti + age:species:ti2,
               data=dat, na.action=na.omit,
               random = list(tree=pdDiag(~1+ti)),
               cor = corExp(form=~ day|tree) )

    m3l # Match Schabenberger output 7.27.  Same fixef, ranef, variances, exp corr

    # vc(m3l)
    ##       effect variance stddev
    ##  (Intercept)  0.1549  0.3936
    ##           ti  0.02785 0.1669
    ##     Residual  0.16    0.4

    # --- asreml
    
    dat <- dat[order(dat$tree,dat$day),]
    m3a <- asreml(water ~ 0 + age:species + age:species:ti + age:species:ti2,
                  data=dat,
                  random = ~ age:species:tree + age:species:tree:ti,
                  residual = ~ tree:exp(day) )
    # code for asremlr 4
    ## m3a <- asreml(water ~ 0 + age:species + age:species:ti + age:species:ti2,
    ##               data=dat,
    ##               random = ~ age:species:tree + age:species:tree:ti,
    ##               resid = ~ tree:exp(day) )
    
    # vc(m3a) # Note: day.pow = .8091 = exp(-1/4.7217)
    ##                       effect component std.error z.ratio constr
    ##     age:species:tree!age.var   0.1549   0.07192      2.2    pos
    ##  age:species:tree:ti!age.var   0.02785  0.01343      2.1    pos
    ##                   R!variance   0.16     0.008917    18      pos
    ##                    R!day.pow   0.8091   0.01581     51    uncon

  }
  

[lloyddewit]

I think I have the problem(s) isolated. There are 2 in this second code:
a) I did need to comment out the if(0){ together with the closing bracket.
b) I also had to comment out lines with a square bracket. for example: dat <- dat[order(dat$tree,dat$day),]

@rdstern yes, you are correct.
Part a is covered by lloyddewit/RScript#19.
Part b is covered by lloyddewit/RScript#18 (note: RScript already parses square brackets but it cannot yet parse [, or ,]).

In order to execute statement-by-statement using ctrl-enter, the whole script needs to be parsed by RScript. If any part of the script can't be parsed, then the user needs to select the part to be run or the user needs to select Run All.

I can see that this could be annoying. To change this, I would need to spend time on improving the back up system. I would prefer to invest the time in RScript so that the backup system is required less and less (hopefully at one point never or extremely rarely).

The question is: could the current approach be merged as-is or would you prefer to wait for a new RScript version before merging?
Could we discuss when we talk on 19 Oct?

[rdstern]

@lloyddewit I am a believer in merging as soon as possible, particularly as I don't see any obvious regression here in other, seemingly unrelated parts of R-Instat.
Your script windows are looking so good, that I want to include them as soon as you feel prepared for this.
In some ways I am also quite happy that you have these problems! (Sorry I should rephrase that!) But this is showing that you are doing something that is complicated and difficult. And that helps me to understand why the other GUIs have not even tried.

Getting to the practical issues I will be happy for this one to be merged and then just need to add the documentation on detecting scripts with possible problems and then describing the work-arounds in those cases.

I see there will be two types of work around.
a) I can run the whole script, but only in complete chunks.
b) There is code in the script that can't be run, so we need to edit the actual code.

An example of b) used to be when there was = instead of <-, used as assignment. I think that's now ok. If I understand your message above correctly, then [, or ,] might be a current instance of b)?

I would at least be keen to know what causes b) and minimise or eliminate those. Perhaps the comma at the start of a multiline is still there, or is that now ok, if I mark the whole section?

I am not even asking for that, before we merge.

Then I could recommend that users "test" a script initially by trying the library command - as I did. If that is ok then all is simple. If not, then they can either run in chunks, or comment out the offending sections. That's all fine!

So I tried a script with a single problem. It then gave a message - as I expected - with this line:

dat <- dat[order(dat$tree, dat$dir), ]

I pressed OK and it seems ok then. So this problem sort of becomes an a) rather than a b) above. So perhaps fine already!? But it didn't give output, so I am not sure if it ran?

Finally an unrelated question. I have been commenting out different blocks of code - so putting a # at the start of multiple lines. Is there a way of doing that easily on a block - perhaps via a right-click?

It would be an R-Instat of this:

"The easiest way to create a multi-line comment in RStudio is to highlight the text and press Ctrl + Shift + C. You can just as easily remove the comment by highlighting the text again and pressing Ctrl + Shift + C"?

[lloyddewit]

@rdstern Thank you for the positive feedback!

I see there will be two types of work around.
a) I can run the whole script, but only in complete chunks.
b) There is code in the script that can't be run, so we need to edit the actual code.

An example of b) used to be when there was = instead of <-, used as assignment. I think that's now ok. If I understand your message above correctly, then [, or ,] might be a current instance of b)?

Some code only works using RScript (e.g. = as assignment), and some code only works using the heuristics approach (e.g. key words and [, or ,]). The user normally only needs to edit the actual code if a script contains both types (otherwise we can get the code to run by using the either the heuristics or RScript approach).

There may also be some R statements that do not work using either approach. I think these are mainly related to how R-Instat handles the output. An example is lloyddewit/RScript#17.

As I mentioned before, I think the best approach is to fix these problems in RScript so that the heuristics approach is needed less and less.

Perhaps the comma at the start of a multiline is still there, or is that now ok, if I mark the whole section?

I think this works using both approaches, do you have a test case that fails?

dat <- dat[order(dat$tree, dat$dir), ]

This only currently works using the heuristics approach. There is no output because it is an assignment statement.

I raised #8568 for commenting blocks of code, and added it to the list in #7989.

I hope this makes sense, let me know if you have any questions.
Thanks!

[lloyddewit]

@rdstern
This PR is ready for your testing/approval.
This will be hard to test because it's so open ended. Based on my knowledge of the code and where weaknesses could be, statement-by-statement testing could include:

• Ensuring different types of output is as expected
• Code that can't be parsed by RScript is handled correctly
• Multiple ststements on one line (seprated by ;)
• Statements split over multiple lines
• Comments at start of statement and/or mixed in with multi-line statements
• Ensuring that cursor moves to start of next statement (handling multiple lines, white space, indents, comments etc)
• Correct statement executes regardless of whether cursor is at start/middle/end of statement; regardless of multiple lines, white space, indents, comments etc.
• empty scripts, single-line scripts (with and without newline at end), large scripts
• statements that take a long time to execute

and whatever else you can think of ...
Thanks!

P.S. Code that can't be parsed by RScript is covered here. Fixing this will be my next task. If anything is missing, then please let me know

[lloyddewit]

@rdstern I fixed the issue with commas inside square bracket operators (e.g. res <- MCA (poison[,3:8],excl=c(1,3))). Please could you test?
Thanks

[lloyddewit]

@lloyddewit I don't know if it is a right place to report this. When I run this I get the same command in the output

@N-thony Thank you for reporting. I think you may be running an older version of this branch. The output from the latest version is shown below. I think this is correct. If I misunderstood, then please let me know.
Thanks

[rdstern]

@lloyddewit great that seems to work fine now.

I am really enjoying running the script statement by statement, through just pressing the Run repeatedly.

The script below has a number of items of interest!
a) Square brackets - working fine.
b) Simple loop with braces (curly brackets). I have commented that out in the code.
c) A package we don't have - it is commercial, so I can't simply download it. Not your problem and it runs everything it can.
d) Comment lines that don't copy to the output.

So I have 2 questions now:

1. Does the editor have a Find/Replace feature? If so, could it be implemented? I tried <ctrl>F and it loaded an irrelevant dialog!
2. I'm not getting all the comments in the Output window. I have a very simple solution to this. I can copy a comment (or anything ) in your script window. Then I can paste it into (say) a new script window. But I can't paste it into the bottom of the output window? Is there a good reason to avoid Patrick adding the facility to paste from the clipboard to the bottom of the output window?

  library(agridat)
  data(harris.wateruse)
  dat <- harris.wateruse

  # Compare to Schabenberger & Pierce, fig 7.23
  libs(latticeExtra)
  useOuterStrips(xyplot(water ~ day|species*age,dat, as.table=TRUE,
                        group=tree, type=c('p','smooth'),
                        main="harris.wateruse 2 species, 2 ages (10 trees each)"))


  # Note that measurements on day 268 are all below the trend line and
  # thus considered outliers.  Delete them.
  dat <- subset(dat, day!=268)


  # Schabenberger figure 7.24
  xyplot(water ~ day|tree,dat, subset=age=="A2" & species=="S2",
         as.table=TRUE, type=c('p','smooth'),
         ylab="Water use profiles of individual trees",
         main="harris.wateruse (Age 2, Species 2)")
  
  # Rescale day for nicer output, and convergence issues, add quadratic term
  dat <- transform(dat, ti=day/100)
  dat <- transform(dat, ti2=ti*ti)

  # Start with a subgroup: age 2, species 2
  d22 <- droplevels(subset(dat, age=="A2" & species=="S2"))

  # ----- Model 1, for subgroup A2,S2

  # First, a fixed quadratic that is common to all trees, plus
  # a random quadratic deviation for each tree.
  
  ## Schabenberger, Output 7.26
  ## proc mixed;
  ##   class tree;
  ##   model water = ti ti*ti / s;
  ##   random intercept ti ti*ti/subject=tree;

  libs(nlme,lucid)
  ## We use pdDiag() to get uncorrelated random effects
  m1n <- nlme::lme(water ~ 1 + ti + ti2, data=d22, na.action=na.omit,
             random = list(tree=nlme::pdDiag(~1+ti+ti2)))
  vc(m1n)
  ##       effect variance    stddev
  ##  (Intercept)   0.2691 0.5188
  ##           ti   0      0.0000144
  ##          ti2   0      0.0000039
  ##     Residual   0.1472 0.3837

  #if(0){  
    # Various other models with lme4 & asreml
    
    libs(lme4, lucid)
    m1l <- lmer(water ~ 1 + ti + ti2 + (1|tree) +
                  (0+ti|tree) + (0+ti2|tree), data=d22)
    vc(m1l)
    ##      grp        var1 var2   vcov  sdcor
    ##     tree (Intercept) <NA> 0.2691 0.5188
    ##   tree.1          ti <NA> 0      0
    ##   tree.2         ti2 <NA> 0      0
    ## Residual        <NA> <NA> 0.1472 0.3837


    # Once the overall quadratic trend has been removed, there is not
    # too much evidence for consecutive observations being correlated
    ## d22r <- subset(d22, !is.na(water))
    ## d22r$res <- resid(m1n)
    ## xyplot(res ~ day|tree,d22r,
    ##        as.table=TRUE, type=c('p','smooth'),
    ##        ylab="residual",
    ##        main="harris.wateruse - Residuals of individual trees")
    ## op <- par(mfrow=c(4,3))
    ## tapply(d22r$res, d22r$tree, acf)
    ## par(op)
    
    # ----- Model 2, add correlation of consecutive measurements
    
    ## Schabenberger (page 516) adds correlation.
    ## Note how the fixed quadratic model is on the "ti = day/100" scale
    ## and the correlated observations are on the "day" scale.  The
    ## only impact this has on the fitted model is to increase the
    ## correlation parameter by a factor of 100, which was likely
    ## done to get better convergence.
    
    ## proc mixed data=age2sp2;
    ##   class tree;
    ##   model water = ti ti*ti / s ;
    ##   random intercept /subject=tree s;
    ##   repeated /subject=tree type=sp(exp)(day);

    ## Same as SAS, use ti for quadratic, day for correlation
    m2l <- lme(water ~ 1 + ti + ti2, data=d22,
               random = ~ 1|tree,
               cor = corExp(form=~ day|tree),
               na.action=na.omit)
    m2l # Match output 7.27.  Same fixef, ranef, variances, exp corr

    # vc(m2l)
    ##       effect variance stddev
    ##  (Intercept)   0.2656 0.5154
    ##     Residual   0.1541 0.3926

    # ---

    ## Now use asreml.  When I tried rcov=~tree:exp(ti),
    ## the estimated parameter value was on the 'boundary', i.e. 0.
    ## Changing rcov to the 'day' scale produced a sensible estimate
    ## that matched SAS.
    ## Note: SAS and asreml use different parameterizations for the correlation
    ## SAS uses exp(-d/phi) and asreml uses phi^d.
    ## SAS reports 3.79, asreml reports 0.77, and exp(-1/3.7945) = 0.7683274
    ## Note: normally a quadratic would be included as 'pol(day,2)'

    libs(asreml)
    d22 <- d22[order(d22$tree, d22$day),]
    m2a <- asreml(water ~ 1 + ti + ti2,
                  data=d22,
                  random = ~ tree,
                  residual=~tree:exp(day))
    vc(m2a)
    ##         effect component std.error z.ratio constr
    ##  tree!tree.var    0.2656   0.1301      2      pos
    ##     R!variance    0.1541   0.01611     9.6    pos
    ##      R!day.pow    0.7683   0.04191    18    uncon


    # ----- Model 3. Full model for all species/ages.  Schabenberger p. 518

    ## /* Continuous AR(1) autocorrelations included      */
    ## proc mixed data=wateruse;
    ##   class age species tree;
    ##   model water = age*species age*species*ti age*species*ti*ti / noint s;
    ##   random intercept ti / subject=age*species*tree s;
    ##   repeated / subject=age*species*tree type=sp(exp)(day);
    

    m3l <- lme(water ~ 0 + age:species + age:species:ti + age:species:ti2,
               data=dat, na.action=na.omit,
               random = list(tree=pdDiag(~1+ti)),
               cor = corExp(form=~ day|tree) )

    m3l # Match Schabenberger output 7.27.  Same fixef, ranef, variances, exp corr

    # vc(m3l)
    ##       effect variance stddev
    ##  (Intercept)  0.1549  0.3936
    ##           ti  0.02785 0.1669
    ##     Residual  0.16    0.4

    # --- asreml
    
    dat <- dat[order(dat$tree,dat$day),]
    m3a <- asreml(water ~ 0 + age:species + age:species:ti + age:species:ti2,
                  data=dat,
                  random = ~ age:species:tree + age:species:tree:ti,
                  residual = ~ tree:exp(day) )
    # code for asremlr 4
    ## m3a <- asreml(water ~ 0 + age:species + age:species:ti + age:species:ti2,
    ##               data=dat,
    ##               random = ~ age:species:tree + age:species:tree:ti,
    ##               resid = ~ tree:exp(day) )
    
    # vc(m3a) # Note: day.pow = .8091 = exp(-1/4.7217)
    ##                       effect component std.error z.ratio constr
    ##     age:species:tree!age.var   0.1549   0.07192      2.2    pos
    ##  age:species:tree:ti!age.var   0.02785  0.01343      2.1    pos
    ##                   R!variance   0.16     0.008917    18      pos
    ##                    R!day.pow   0.8091   0.01581     51    uncon

  #}
  

[rdstern]

@Patowhiz hope you can check this. It is still "work in progress" but good for you to examine, and then consider the remaining work as a new later pull request.

[lloyddewit]

@rdstern Thank you for testing.

1. Does the editor have a Find/Replace feature?

No, this is already requested in issue #7989.

2. I'm not getting all the comments in the Output window.

I tested and, as far as I can see, all the comments are displayed in the output window. The only exception is when a comment is not followed by an executable statement (i.e. comments at the end of the script); or when only a comment (with no executable R script) is selected and run. These comments are ignored (deliberate design choice). Would you like comments without statements to also be sent to the Output window?
As a temporary workaround you can just add a minimal dummy statement (e.g. a 1) after the final comment, and execute that (see below).

If I misunderstood anything, then please let me know.
Thanks

[rdstern]

@lloyddewit you seem to imply that would be easy to do. When I mark and run a comment then yes, it would be great if it could be sent to the output window.

Meantime I am happy with your dummy command.
The other comment that is not sent to the output window is when it is on the same line as a command that produces a result. I'll give an example below. Again, I am quite ok with that, but if I deliberately copy and run that comment, then (again) it would be nice if that simply copied to the output window.

So here is an example with comments at the end of lines:

Here are 2 lines from this:

vp <- s2m + s2f + s2mf + s2w  # variability of phenotypes = 1.3869
  4*s2m / vp # heritability male 0.346

The first, is an assignment and the comment is in the output window.
The second gives the result in the output window, but (of course?) without the comment.

library(agridat)
 data(hanover.whitepine)
 dat <- hanover.whitepine

 libs(lattice)
 # Relatively high male-female interaction in growth comared
 # to additive gene action. Response is more consistent within
 # male progeny than female progeny.
 # with(dat, interaction.plot(female, male, length))
 # with(dat, interaction.plot(male, female, length))
 bwplot(length ~ male|female, data=dat,
        main="hanover.whitepine - length for male:female crosses",
        xlab="Male parent", ylab="Epicotyl length")
 
 # Progeny sums match Becker p 83
 sum(dat$length) # 380.58
 aggregate(length ~  female + male, data=dat, FUN=sum)
 
 # Sum of squares matches Becker p 85
 m1 <- aov(length ~ rep + male + female + male:female, data=dat)
 anova(m1)
 
 # Variance components match Becker p. 85
 libs(lme4)
 libs(lucid)
 m2 <- lmer(length ~ (1|rep) + (1|male) + (1|female) + (1|male:female), data=dat)
 #as.data.frame(lme4::VarCorr(m2))
 vc(m2)
 ##         grp        var1 var2    vcov  sdcor
 ## male:female (Intercept) <NA> 0.1369  0.3699
 ##      female (Intercept) <NA> 0.02094 0.1447
 ##        male (Intercept) <NA> 0.1204  0.3469
 ##         rep (Intercept) <NA> 0.01453 0.1205
 ##    Residual        <NA> <NA> 0.2004  0.4477
 
 # Becker used this value for variability between individuals, within plot
 s2w <- 1.109
 
 # Calculating heritability for individual trees
 s2m <- .120
 s2f <- .0209
 s2mf <- .137
 vp <- s2m + s2f + s2mf + s2w  # variability of phenotypes = 1.3869
 4*s2m / vp # heritability male 0.346
 4*s2f / vp # heritability female 0.06
 2*(s2m+s2f)/vp # heritability male+female .203
 # As shown in the boxplot, heritability is stronger through the
 # males than through the females.

[lloyddewit]

@rdstern
I could not recreate the commenting problem (see below). Please could you demonstrate it in our meeting?
Thanks

[rdstern]

@lloyddewit after our discussion it is clear I made a mistake on that point, i.e. on the comment not appearing sometimes when it was at the end of an executable line. I had expected it to appear after the result. Instead it does appear, but above where I looked, as part of the command line, and that is quite right!

[lloyddewit]

@rdstern You reported that comments at the end of scripts, that are not associated with any R statement, are not shown in the output window. On 17 Oct, you gave this example:

I have now changed this to what you requested. You can use ctrl-enter to execute comments appended to a script; or you can highlight/execute just a comment. Run All will also show any final script comments in the output window.

I also fixed the bug shown below.

Please could you test/approve?
Thanks

[lloyddewit]

@rdstern I also marked the following issue #7989 task as done. Please could you test that you agree? Thanks

[rdstern]

@Patowhiz this still all seems good.
Note the issue of the log window opening is fixed in Antoine's pull request.

[Patowhiz]

@lloyddewit thanks for this.
Below are my few requests and suggestions.

[Patowhiz]

@lloyddewit thanks for addressing my suggestions.
Once you refactor the functions to avoid the duplication we can merge.

[lloyddewit]

@lloyddewit thanks for addressing my suggestions. Once you refactor the functions to avoid the duplication we can merge.

@Patowhiz Thanks. I replaced some of the duplicated code with shared private functions. I tried to refactor out more of the duplicated code but found that to make it work, I would need to significantly refactor the existing RunScript() function. This would be a significant task. As you know, this function is called by all the dialogs so there would be a high risk of regression. Maybe we should do this one day but I prefer not to add such a significant task to this PR.
The remaining duplication is mainly between RunScript() and RunScriptFromWindow(). The latter function is only a backup and is called when RScript raises an exception. I am working on solving these exceptions in the RScript library. We should be able to remove RunScriptWindow() in the next weeks/months. So if we leave the refactoring at the current level, then I don't think we are creating technical debt.
What's your view?

[rdstern]

I'm approving again. @Patowhiz hope it is ok to merge now, so we can proceed with #8597

[Patowhiz]

@rdstern @lloyddewit give me more time to look into this.
I'm still looking into PR #8597 and it's issues, which are not as trivial as @rdstern would have wished them to be.

[ChrisMarsh82]

@Patowhiz Can this be merged as the code looks fine? Then if there are more changes they can be done on #8597