serve-index icon indicating copy to clipboard operation
serve-index copied to clipboard

Add `simplifyFileSize` option

Open KroshSputnik opened this issue 10 months ago • 10 comments

I added a new option to enable to show the files sizes in B, MB, GB, TB, PB.

Enable it in the options by doing 'simplifyFileSize':true

Not sure if I need to divide the size by 1024 or 1000. Also would I need to round the final size, as it seems the size returned for the file size is already rounded. Feedback will be appreciated.

KroshSputnik avatar Jan 29 '25 10:01 KroshSputnik

Currently we support [email protected] so new ES6 syntax is not allowed:

> [email protected] test /home/runner/work/serve-index/serve-index
> mocha --reporter spec --bail --check-leaks test/


/home/runner/work/serve-index/serve-index/index.js:308
      const fileSizes = ['B', 'KB', 'MB', 'GB', 'TB', 'PB']
      ^^^^^
SyntaxError: Use of const in strict mode.
    at Module._compile (module.js:439:25)
    at Object.Module._extensions..js (module.js:474:[10](https://github.com/expressjs/serve-index/actions/runs/13029380620/job/36428408044?pr=114#step:10:11))
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (/home/runner/work/serve-index/serve-index/test/test.js:8:18)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at /home/runner/work/serve-index/serve-index/node_modules/mocha/lib/mocha.js:220:27
    at Array.forEach (native)
    at Mocha.loadFiles (/home/runner/work/serve-index/serve-index/node_modules/mocha/lib/mocha.js:217:14)
    at Mocha.run (/home/runner/work/serve-index/serve-index/node_modules/mocha/lib/mocha.js:469:10)
    at Object.<anonymous> (/home/runner/work/serve-index/serve-index/node_modules/mocha/bin/_mocha:404:18)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:[11](https://github.com/expressjs/serve-index/actions/runs/13029380620/job/36428408044?pr=114#step:10:12)9:16)
    at node.js:945:3
npm ERR! weird error 8
npm ERR! not ok code 0

Full log

UlisesGascon avatar Jan 30 '25 16:01 UlisesGascon

Thanks for this proposal @KroshSputnik! Can you include tests and documentation?

Hi @UlisesGascon ~~I'm not sure how to add a test, do I need to create a new one in test.js?~~ ~~I have never created one before, could you walk it through?~~

I've added docs in readme and fixed the es6 issue.

Thanks

KroshSputnik avatar Jan 31 '25 03:01 KroshSputnik

@UlisesGascon I have added test, for when the option is enabled and when it isnt.

Can you double check my pull request.

KroshSputnik avatar Feb 01 '25 12:02 KroshSputnik

It looks great, although we could probably delegate that logic to bytes instead of handling it ourselves.

bjohansebas avatar Feb 01 '25 15:02 bjohansebas

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/[email protected] None 0 12.3 kB dougwilson

View full report↗︎

socket-security[bot] avatar Feb 01 '25 22:02 socket-security[bot]

@bjohansebas Updated the feature with your suggestion.

KroshSputnik avatar Feb 01 '25 22:02 KroshSputnik

Great! Once the tests are run, I'll be able to give my LGTM

bjohansebas avatar Feb 01 '25 22:02 bjohansebas

@UlisesGascon All tasks completed. If need anything else, let me know.

KroshSputnik avatar Feb 02 '25 10:02 KroshSputnik

mmm.. not sure why one of the new tests is failing :thinking:. Can you have a look @KroshSputnik ?

UlisesGascon avatar Feb 02 '25 15:02 UlisesGascon

I think the issues due to the differences of how linux and windows calculate bytes.

I downgraded to node 0.8, but all tests pass successfully.

KroshSputnik avatar Feb 03 '25 12:02 KroshSputnik