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

Change config file location + Small fixes #16

Merged
merged 4 commits into from
Apr 29, 2023
Merged

Change config file location + Small fixes #16

merged 4 commits into from
Apr 29, 2023

Conversation

gAbelli
Copy link
Contributor

@gAbelli gAbelli commented Apr 18, 2023

Hi! I worked on some small changes and I wanted to propose them to you. Feel free to reject them or to suggest improvements. Here is what I did.

  • I didn't particularly like to have the configuration file in ~/.gptrc for two reasons: (1) I don't like polluting my home folder too much and (2) the "rc" that you see for example in "bashrc" stands for "run commands" and is usually meant to identify a file that can be executed by a shell, but in our case the file is a yaml configuration file. So I moved it to ~/.config/gpt-cli/gpt.yml as it is quite common.
  • I consequently updated the documentation, also adding information on how to run the program from anywhere.
  • I also made a slight change in the way python handles the path to make it compatible with Windows.
  • I added missing arguments in a formatted string.

Thank you for the great project, I hope I can contribute more in the future!

@kharvd
Copy link
Owner

kharvd commented Apr 28, 2023

Thanks for working on this! This is valuable feedback.

As is, this change is not backwards-compatible: it will break someone's config if they pull the repo. I wonder if we can fall back to using .gptrc if it exists

@gAbelli
Copy link
Contributor Author

gAbelli commented Apr 28, 2023

Thank you for your reply!
Yes I would say it's totally possible to make it check for both files, giving priority to ~/.config/gpt-cli/gpt.yml. I'll work on this later and update the pull request.

@gAbelli
Copy link
Contributor Author

gAbelli commented Apr 28, 2023

It should be good now. I tested it on both Linux and Windows.
Let me know if you find more things to improve!

Copy link
Owner

@kharvd kharvd left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@kharvd kharvd merged commit 7752b05 into kharvd:main Apr 29, 2023
1 check passed
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