Closed mkfmnn closed 2 months ago
only small issue I have is the string error type
I initially did try using ScannerError but the CLI UX felt messy (no reason to be showing the source location of the error) so I more preferred emulating the clap
errors—though I stopped short of reworking the command-line parsing to give me access to the Command
to hook into clap's error reporting.
Let me know if you have any references/reading for the pull request workflow or commit/coding conventions you'd like me to follow for the future. I tried to emulate what it looked like you've been doing and it feels like a standard process that other devs use too but I don't know if there's an official standard for it somewhere.
I initially did try using ScannerError but the CLI UX felt messy (no reason to be showing the source location of the error) so I more preferred emulating the
clap
errors—though I stopped short of reworking the command-line parsing to give me access to theCommand
to hook into clap's error reporting.
I want to get rid of error-stack
and just switch to thiserror
in the scanner crate aswell. I've never hooked into clap's error reporting but if thats a thing then that could be quite nice to have for the cases where more args are required yeah.
Let me know if you have any references/reading for the pull request workflow or commit/coding conventions you'd like me to follow for the future. I tried to emulate what it looked like you've been doing and it feels like a standard process that other devs use too but I don't know if there's an official standard for it somewhere.
I don't have a contributing.md or anything along those lines. I mostly just try to stick to conventional commits and thats about it in terms of procedure. Branch naming is up to you and your commits while working on a PR are also not too important since they'll be squashed before merging anyways.
also: I have no idea why the code scanning CI job is taking forever...
--factorio-userdir
--factorio
optional (could arguably be renamed to--factorio-appdir
but I didn't want to break backward compatibility)--factorio-*
command-line options correctly for default macOS installation locations (and Linux, according to the wiki)On my Mac this lets me run the scanner with just the command
scanner --out x.png file bp.txt