bescoto commented 5 years ago

Excel names don't need to point to regions, they may point to constants, such as 10000, or other names. Currently getNamedRegions can skip these, causing the sheets and regions attributes not to line up with the names. This patch just sets the sheet and region to "" (empty string) for the names like this.

codecov[bot] commented 5 years ago

Codecov Report

Merging #474 into master will increase coverage by 0.08%. The diff coverage is 95.23%.

@@            Coverage Diff             @@
##           master     #474      +/-   ##
==========================================
+ Coverage   60.23%   60.32%   +0.08%     
==========================================
  Files          30       30              
  Lines        7142     7155      +13     
==========================================
+ Hits         4302     4316      +14     
+ Misses       2840     2839       -1

Impacted Files	Coverage Δ
R/wrappers.R	`42.7% <100%> (+0.09%)`	:arrow_up:
R/readWorkbook.R	`83.33% <100%> (+0.23%)`	:arrow_up:
R/helperFunctions.R	`75.86% <100%> (+0.3%)`	:arrow_up:
R/loadWorkbook.R	`69.97% <80%> (+0.41%)`	:arrow_up:

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 ead0038...11168d3. Read the comment docs.

ycphs commented 4 years ago

I implemented your PR in my new fork of the repository https://github.com/ycphs/openxlsx

awalker89 / openxlsx

Improve handling of non-region names in getNamedRegions and add related test #474

Codecov Report