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: adds utility service and access to commander #251

Closed
wants to merge 0 commits into from

Conversation

jmcdo29
Copy link
Owner

@jmcdo29 jmcdo29 commented Jan 26, 2022

There is a new CliUtilityService and @InjectCommnder() decoraotr
that allows for direct access to the commander instnace. The utility service
has methods like parseBoolean, parseInt, and parseFloat. The
number parsing methods are just simple wrappers around Number.parse*(),
but the boolean parsing method handles true being yes, y, 1,
true, and t and false being no, n, false, f, and 0.

@nx-cloud
Copy link

nx-cloud bot commented Jan 26, 2022

Nx Cloud Report

CI is running for commit 4d93adb.

📂 Click to track the progress, see the status, the terminal output, and the build insights.


Sent with 💌 from NxCloud.

@changeset-bot
Copy link

changeset-bot bot commented Jan 26, 2022

⚠️ No Changeset found

Latest commit: 4d93adb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@jmcdo29
Copy link
Owner Author

jmcdo29 commented Jan 26, 2022

@micalevisk @smolinari would y'all mind taking a look at this and checking my logic along with what the addition is? Thanks guys

@micalevisk
Copy link
Contributor

I'll try this feat out next saturday. The logic seems fine.


I think would be cool to add in-code docs like

/**
 * A helper method to turn a string (case insensitive) into a boolean based on true or false values.
 *
 * Any string that matches one of the following values is considered a `true` input:
 * - `yes`
 * - `y`
 * - `true`
 * - `t`
 * - `1`
 *
 * Any string that matches one of the following values is considered a `false` input:
 * - `no`
 * - `n`
 * - `false`
 * - `f`
 * - `0`
 *
 * @param val
 * @throws {Error} If `val` is not a _true_ nor _false_ expression
 */
parseBoolean(val: string): boolean {

and

/**
 * A Simple wrapper around `Number.parseInt()`
 * @param val
 * @param radix
 */
parseInt(val: string, radix = 10): number {

/**
 * A simple wrapper around `Number.parseFloat()`
 * @param val
 */
parseFloat(val: string): number {

to improve DX a bit. Although this is redundant as they're documented already on the docs site.

@jmcdo29
Copy link
Owner Author

jmcdo29 commented Jan 27, 2022

I'll try this feat out next saturday. The logic seems fine.

I think would be cool to add in-code docs like

/**
 * A helper method to turn a string (case insensitive) into a boolean based on true or false values.
 *
 * Any string that matches one of the following values is considered a `true` input:
 * - `yes`
 * - `y`
 * - `true`
 * - `t`
 * - `1`
 *
 * Any string that matches one of the following values is considered a `false` input:
 * - `no`
 * - `n`
 * - `false`
 * - `f`
 * - `0`
 *
 * @param val
 * @throws {Error} If `val` is not a _true_ nor _false_ expression
 */
parseBoolean(val: string): boolean {

and

/**
 * A Simple wrapper around `Number.parseInt()`
 * @param val
 * @param radix
 */
parseInt(val: string, radix = 10): number {

/**
 * A simple wrapper around `Number.parseFloat()`
 * @param val
 */
parseFloat(val: string): number {

to improve DX a bit. Although this is redundant as they're documented already on the docs site.

Yeah, I think the doc string would definitely be helpful for in IDE docs. It's just that none of the other methods have that, so adding it might feel out of place.

@smolinari
Copy link
Contributor

From what I can see, looks good. 👍 Asking me to check your work is like master Po asking Grasshopper if his Kung Fu is good. LOL! 😁

Scott

@micalevisk
Copy link
Contributor

@jmcdo29 how do we import this new service?

it isn't listed below

image

@jmcdo29
Copy link
Owner Author

jmcdo29 commented Jan 29, 2022

Whoops, missed that. See this is why I have someone double check me

@jmcdo29 jmcdo29 closed this Jan 29, 2022
@jmcdo29 jmcdo29 force-pushed the feat/cli-util-service branch from 81da92a to 4d93adb Compare January 29, 2022 19:55
@jmcdo29 jmcdo29 deleted the feat/cli-util-service branch September 4, 2023 00:04
@coler-j
Copy link

coler-j commented Apr 22, 2024

How do we use this?

@jmcdo29
Copy link
Owner Author

jmcdo29 commented Apr 22, 2024

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.

4 participants