julia-vscode / DocumentFormat.jl

Auto-formatter for Julia
Other
62 stars 18 forks source link

Replace sizeof with lastindex #75

Closed davidanthoff closed 4 years ago

davidanthoff commented 4 years ago

Those are all instances of sizeof in this package. I'm fairly positive that they should all be replaced with lastindex, but not 100% sure, so would be great if @ZacLN could review carefully :)

codecov[bot] commented 4 years ago

Codecov Report

Merging #75 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master     #75   +/-   ##
======================================
  Coverage    77.8%   77.8%           
======================================
  Files           4       4           
  Lines         401     401           
======================================
  Hits          312     312           
  Misses         89      89
Impacted Files Coverage Δ
src/utils.jl 58.82% <100%> (ø) :arrow_up:
src/passes.jl 71.09% <100%> (ø) :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 ce2195a...6d0ed6a. Read the comment docs.

ZacLN commented 4 years ago

I'm pretty sure edits are measured against and made to the underlying UInt8 array (https://github.com/julia-vscode/DocumentFormat.jl/blob/master/src/DocumentFormat.jl#L181-L184) so sizeof should be applicable here. I've lost sense of how this all works so am slightly wary of making changes if its not in response to an actual error/crash

davidanthoff commented 4 years ago

Hm, but at least the change for utils.jl is definitely on a String, right?

ZacLN commented 4 years ago

I suppose so, lets give it a shot