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

Trim shell prefix #18

Merged
merged 3 commits into from
Mar 22, 2024
Merged

Trim shell prefix #18

merged 3 commits into from
Mar 22, 2024

Conversation

hanchchch
Copy link
Contributor

@hanchchch hanchchch commented Mar 21, 2024

Hi, thank you for building and publishing this fascinating cli app.
I had a tiny issue with suggest command. llama puts a shell prefix such as $ or sometimes, like below.

❯ tlm s 'create secret'> Thinking... (3.875119084s)
┃ > $ openssl rand -base64 32
┃ > Executing...

bash: $: command not found

It's not very often but it's disrupting the experience since you can't execute the command right after the suggestion when it happens.

I made a small solution simply by trimming the shell prefix if exists. I think it can be more developed like adding below method.

func (s *Suggest) refineCommand(command string) string {
  // refine the generated command, such as trimming shell prefix or removing possible mistakes
}

Because the generated commands are not always perfect, it would be helpful to handle known/possible issues.

@yusufcanb
Copy link
Owner

Thanks for your kind words and your contribution. Would you be interested in writing a unit test for func (s *Suggest) refineCommand(command string) string as well? This is what I'm gonna do before approving it.

Copy link

sonarcloud bot commented Mar 21, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@hanchchch
Copy link
Contributor Author

Sure, I just added it. Note that I had to write it in suggest package instead of suggest_test, because the refineCommand method is not exported.

@yusufcanb yusufcanb changed the base branch from main to release/1.2 March 22, 2024 22:35
@yusufcanb yusufcanb merged commit 2908d87 into yusufcanb:release/1.2 Mar 22, 2024
3 checks passed
@yusufcanb
Copy link
Owner

Thank you for your contribution! ❤

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.

None yet

2 participants