int i and nomatch arg

[jangorecki]

closes #3109
closes #3666
also refactors code for better code coverage, thus is likely to get some failure for codcov check if any of those expanded lines were not covered before

[codecov]

Codecov Report

Merging #4353 (27a1c2c) into master (1bc9178) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 27a1c2c differs from pull request most recent head 442e0d3. Consider uploading reports for the commit 442e0d3 to get more accurate results

@@           Coverage Diff           @@
##           master    #4353   +/-   ##
=======================================
  Coverage   99.54%   99.54%           
=======================================
  Files          76       76           
  Lines       14612    14623   +11     
=======================================
+ Hits        14545    14556   +11     
  Misses         67       67           

Impacted Files	Coverage Δ
R/data.table.R	99.94% <100.00%> (+<0.01%)	⬆️
src/bmerge.c	100.00% <100.00%> (ø)
src/nqrecreateindices.c	100.00% <100.00%> (ø)
src/subset.c	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bc9178...442e0d3. Read the comment docs.

[mattdowle]

Both before and after my changes, this PR turns on nomatch=0 as well as nomatch=NULL:

DT = data.table(x = 1:4)
> DT[c(1L, 5L, NA_integer_)]
       x
   <int>
1:     1
2:    NA
3:    NA
> DT[c(1L, 5L, NA_integer_), nomatch=NULL]  # new behavior NEWS item mentions
       x
   <int>
1:     1
> DT[c(1L, 5L, NA_integer_), nomatch=0]  # nomatch=0 can be used as well
       x
   <int>
1:     1

The new tests don't cover nomatch=0, but iiuc and what Jan writes, we want to only allow nomatch=NULL for this new feature since nomatch=0 is only for backwards compatibility as per news item 5 in v1.12.0 (Jan 2019) :

So we don't want to open the door to folk starting to use nomatch=0L for this (which is #3109's request but Jan correctly nudges towards NULL.)

The difficulty to overcome is that [.data.table converts NULL to 0L near the top so that nomatch has two states from then on. If we're going to turn this new feature on only for nomatch=NULL then we might need to carry 3 states through. Or, we could put in the warning that 0L is deprecated as that NEWS item warned (option to turn on), or warn just in this case of i being row numbers because nomatch was ignored before in this case. That way folk trying to use this new feature with nomatch=0L will be nudged towards nomatch=NULL.

[mattdowle]

Merging now to make progress. As always, everything is always open for discussion and reconsideration until release.