Skip to content
This repository has been archived by the owner on Jun 24, 2024. It is now read-only.

WIP: Refactor Cli #74

Closed
wants to merge 5 commits into from
Closed

WIP: Refactor Cli #74

wants to merge 5 commits into from

Conversation

hhamud
Copy link
Contributor

@hhamud hhamud commented Mar 25, 2023

Refactor and clean up of the Command-line interface.

This PR aims to decouple prompt generation from model generation and make it easier to expand upon in the future.

TODO:

  • Decouple prompt generation from model loading and
  • Separate CLI UI from business logic
  • Create separate modes for the CLI

@hhamud hhamud marked this pull request as draft March 25, 2023 19:40
@philpax
Copy link
Collaborator

philpax commented Mar 25, 2023

You'll probably want to wait until the other PRs land

@hhamud
Copy link
Contributor Author

hhamud commented Mar 25, 2023

You'll probably want to wait until the other PRs land

Which ones in particular are you thinking of?

@philpax
Copy link
Collaborator

philpax commented Mar 25, 2023

#41, #68, #72, and once those are in I'm going to do some cleanup work in preparation for #57

@philpax
Copy link
Collaborator

philpax commented Mar 26, 2023

Getting closer to being able to unblock you on this. In the meantime, you may be interested in looking into revising the CLI parameters to accommodate the different operational modes we have here; I suggest using Clap subcommands.

As an example of how you can share those parameters, here's how I do it in one of my other projects: https://github.com/AmbientRun/Ambient/blob/main/app/src/cli/mod.rs#L7-L100

(This should also help with offering a mode for #40)

@hhamud
Copy link
Contributor Author

hhamud commented Mar 26, 2023

Getting closer to being able to unblock you on this. In the meantime, you may be interested in looking into revising the CLI parameters to accommodate the different operational modes we have here; I suggest using Clap subcommands.

As an example of how you can share those parameters, here's how I do it in one of my other projects: https://github.com/AmbientRun/Ambient/blob/main/app/src/cli/mod.rs#L7-L100

(This should also help with offering a mode for #40)

When I started repl-mode was about merged in and I thought about creating an interactive-mode for the "chat-like" app but I can see there are more on the way.

@hhamud hhamud changed the title Refactor Cli WIP: Refactor Cli Mar 27, 2023
@philpax
Copy link
Collaborator

philpax commented Apr 6, 2023

#83's refactored the CLI a fair bit - do you think there should be more work in that area?

@hhamud
Copy link
Contributor Author

hhamud commented Apr 6, 2023

#83's refactored the CLI a fair bit - do you think there should be more work in that area?

There were a few additional things that I wanted to do after #85 get's merged that would further change the way models are loaded into the CLI.

@philpax
Copy link
Collaborator

philpax commented Apr 7, 2023

Hm, should I close this PR and we'll revisit once #85's in?

@hhamud hhamud closed this Apr 7, 2023
@hhamud hhamud mentioned this pull request Apr 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants