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

refactor: use citty for cli #113

Merged
merged 12 commits into from
Dec 23, 2023
Merged

refactor: use citty for cli #113

merged 12 commits into from
Dec 23, 2023

Conversation

Barbapapazes
Copy link
Member

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

I rewrite CLI to use citty instead of a home made CLI. The only drawback is that we now need to specify "clone" to clone a template from a registry and that's why it's a breaking change.

This will help use to maintain the CLI and add new features more easily. Indeed, we will be able to add new commands to support multiple registries.

Here what could come next:

import { defineCommand } from "citty";

export default defineCommand({
  meta: {
    name: "registry",
    description: "Manage registries",
  },
  subCommands: {
    // eslint-disable-next-line unicorn/prefer-top-level-await
    add: import("./registry/add").then((m) => m.default),
    // eslint-disable-next-line unicorn/prefer-top-level-await
    remove: import("./registry/remove").then((m) => m.default),
  },
});
import { defineCommand } from "citty";
import { updateUser } from "rc9"
import { consola } from "consola"

export default defineCommand({
  meta: {
    name: "add",
    description: "Add a new registry",
  },
  args: {
    name: {
      type: "positional",
      description: "Name of the registry",
    },
    url: {
      type: "positional",
      description: "URL of the registry",
    }
  },
  run: ({ args }) => {
    updateUser({ registries: { [args.name]: args.url } }, '.giget')
    consola.debug(`Added registry ${args.name} with URL ${args.url}`)
  }
})
import { defineCommand } from "citty";
import { consola } from "consola";
import { readUser, writeUser } from "rc9";

export default defineCommand({
  meta: {
    name: "remove",
    description: "Remove a registry",
  },
  args: {
    name: {
      type: "positional",
      description: "Name of the registry",
    },
  },
  run: ({ args }) => {
    const config = readUser(".giget");
    consola.debug('Read config', config)

    delete config.registries[args.name];
    consola.debug('Deleted registry', args.name)

    writeUser(config, ".giget");
    consola.debug('Wrote config', config)
  }
})

Then, with some changes in the internal logic, we could support multiple registries at the same time (related to #112).

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@Barbapapazes Barbapapazes self-assigned this Oct 11, 2023
@Barbapapazes Barbapapazes added the enhancement New feature or request label Oct 11, 2023
@Barbapapazes Barbapapazes requested a review from pi0 October 11, 2023 13:27
@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Attention: 78 lines in your changes are missing coverage. Please review.

Comparison is base (03053bd) 62.45% compared to head (e710ab1) 59.84%.

Files Patch % Lines
src/cli.ts 0.00% 78 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #113      +/-   ##
==========================================
- Coverage   62.45%   59.84%   -2.61%     
==========================================
  Files           7        7              
  Lines         506      528      +22     
  Branches       44       44              
==========================================
  Hits          316      316              
- Misses        189      211      +22     
  Partials        1        1              

β˜” View full report in Codecov by Sentry.
πŸ“’ Have feedback on the report? Share it here.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Member

@pi0 pi0 Oct 11, 2023

Choose a reason for hiding this comment

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

We can avoid breaking change by having it in main. I will help on that in the meanwhile, i think we can call it clone.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've named it "copy" to avoid "clone" which is for git where the git history is also copied.

And yes, I would love some help because I don't know you to avoid breaking change since run is called after a subcommand is called.

src/commands/copy.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@Barbapapazes Barbapapazes requested a review from pi0 October 12, 2023 15:21
@pi0 pi0 changed the title feat!: move cli to citty refactor: use citty for cli Dec 23, 2023
@pi0 pi0 merged commit 10f62bf into main Dec 23, 2023
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants