Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Enhance cli command line #107

Merged
merged 12 commits into from
Jul 5, 2023
68 changes: 58 additions & 10 deletions gbfs-validator/cli.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,65 @@
const { inspect } = require('util')
const commander = require('commander')
const fs = require('fs')
const fsPath = require('path')
const GBFS = require('./')
const pjson = require('./package.json');

if (!process.argv[2]) {
console.error('Usage: gbfs-validator GBFS_URL')
process.exit(1)
getFeedValidationReport = async (url) => {
const gbfs = new GBFS(url)
return gbfs.validation()
}

const gbfs = new GBFS(process.argv[2])
parseOptions = () => {
commander
.version(pjson.version, '-v, --version')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's always 1.0.0 because we don't use it right now.
We return the git hash instead https://github.com/MobilityData/gbfs-validator/blob/master/gbfs-validator/gbfs.js#L3

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in preparation for publishing an npm package. In the case of the command line, if we would like to include the commit hash, more work is needed to make available the hash information on local computers.

.usage('[OPTIONS]...')
.option('-u, --url <dataset_folder>', 'URL of the GBFS feed')
davidgamez marked this conversation as resolved.
Show resolved Hide resolved
.option('-p, --print', 'Print report to console')
.option('-vb, --verbose', 'Verbose mode prints debugging console logs')
.option('-s, --save-report <report_path>', 'Local path to output report file')
.parse(process.argv)

if (require.main === module) {
gbfs
.validation()
.then(result => console.log(inspect(result, { depth: null, colors: true })))
} else {
module.exports = gbfs.validation()
return commander.opts()
}

const options = parseOptions();

const saveReport = (report, filePath) => {
const dirname = fsPath.dirname(filePath);
if (!fs.existsSync(dirname)) {
fs.mkdirSync(dirname, { recursive: true });
}
fs.writeFileSync(filePath, JSON.stringify(report))
}

const processFeedValidation = async () => {
if (options.verbose) {
console.log("Started GBFS validation with options: \n " + inspect(options, { depth: null, colors: true }))
}
try {
const report = await getFeedValidationReport(options.url)
if (options.print) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make it default ?
If no -p / -s arguments is passed, nothing happen, and we don't have any clue

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think changing -p to -nstdout or similar as No standard output, in addition to trigger an error if -nstdout is passed and -s is not present.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, what do you think about removing the -p option completely and only print when option -s is missing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good solution too. My only concern is to drive the use of the command line too much. Having separate options give more control over what goes to a file and stdout.

console.log(inspect(report, { depth: null, colors: true }))
}
if (options.saveReport) {
saveReport(report, options.saveReport)
}
} catch (error) {
console.error(`Critical error while validating GBFS feed => ${error}`)
process.exit(1);
}
}

if (options.url) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this parameter is mandatory, it may be preferable to pass the URL such as node gbfs-validator/cli.js [URL] instead of -u.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll add the option as mandatory. I considered adding -f for local files in the near future; that's why defining -u for URLs and -f for files gives consistency for the consumer.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting for the local files :)

processFeedValidation().then(
() => {
if (options.verbose) {
console.log("Validation completed")
}
}
)
} else {
commander.help()
process.exit(1)
}
1 change: 1 addition & 0 deletions gbfs-validator/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"ajv": "^8.9.0",
"ajv-errors": "^3.0.0",
"ajv-formats": "^2.1.1",
"commander": "^11.0.0",
"fast-json-patch": "^3.1.0",
"got": "^11.8.2",
"json-merge-patch": "^1.0.2"
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3692,6 +3692,11 @@ combined-stream@^1.0.6, combined-stream@^1.0.8:
dependencies:
delayed-stream "~1.0.0"

commander@^11.0.0:
version "11.0.0"
resolved "https://registry.yarnpkg.com/commander/-/commander-11.0.0.tgz#43e19c25dbedc8256203538e8d7e9346877a6f67"
integrity sha512-9HMlXtt/BNoYr8ooyjjNRdIilOTkVJXB+GhxMTtOKwk0R4j4lS4NpjuqmRxroBfnfTSHQIHQB7wryHhXarNjmQ==

commander@^2.3.0, commander@^2.8.1:
version "2.20.3"
resolved "https://registry.yarnpkg.com/commander/-/commander-2.20.3.tgz#fd485e84c03eb4881c20722ba48035e8531aeb33"
Expand Down
Loading