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

NEW: Add omit column feature #46

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Conversation

Inukares
Copy link

@Inukares Inukares commented Mar 25, 2020


Closes #47


First of all, I'd like to say I'm extremely happy to participate in this cool project, great idea @ahmadawais!

This pull request enables feature to omit whole column(s) from output, separated by _.

node index.js --omit=deaths_deaths-today_per-milion

Zrzut ekranu 2020-03-28 o 17 31 16

Wrong keys are simply ignored.

node index.js --o=wrongKeyNothingHappens_deaths

Zrzut ekranu 2020-03-28 o 17 34 56

const states = input === 'states' ? true : false;
const states = input === 'states';
Copy link
Author

Choose a reason for hiding this comment

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

It's the same code written simpler.

Comment on lines -48 to +50
const table = !states
? new Table({ head, style, chars: border })
: new Table({ head: headStates, style, chars: border });
const tableHead = states ? headStates : head;
const table = new Table({ head: tableHead, style, chars: border });
Copy link
Author

Choose a reason for hiding this comment

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

logic for table was complex - it started with negation even though there was no real reason for it. As I needed this piece of code I refactored it a little bit for easier maintenance.

Comment on lines -55 to +57
await getCountry(spinner, table, states, country);
await getStates(spinner, table, states, options);
await getCountries(spinner, table, states, country, options);
await getCountry({ spinner, table, states, country, tableHead });
await getStates({ spinner, table, states, options, tableHead });
await getCountries({ spinner, table, states, country, options, tableHead });
Copy link
Author

Choose a reason for hiding this comment

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

Due to multiple parameters I refactored getCountry, getStates and getCountries to accept object.

@surgeharb
Copy link

It is a good idea to use "comma" separated list as it has common use in cli list parameters
Most of the users will be familiar with this syntax and not to confuse with the hyphens
node index.js --omit=deaths,deaths-today,per-milion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Omit columns from output
5 participants