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

credentials: Use cross-platform credentials.toml instead of Unix-specific .env files. #2

Merged
merged 22 commits into from
Mar 5, 2023

Conversation

A1ex-N
Copy link

@A1ex-N A1ex-N commented Feb 15, 2023

implements #1.

I only started learning V a few days ago so if i've done anything stupid in the code, let me know.

  • Can now load credentials from a toml file (credentials.toml in the same directory as klonol.exe by default. Can change with command-line option -c).
  • get_github_credentials() and get_gittea_credentials() have been merged into a single function

important

I've tested the github features and it's working, but as i'm not familiar with gittea i've encountered issues trying to test it.
I created an account at http://gittea.dev and generated an access token, but when i try to use it i get the error The access token is invalid. I'm not sure what the issue is and i think i'll leave that up to you @hungrybluedev to fix, because i have no idea how to fix it.

A1ex-N added 7 commits February 15, 2023 12:27
get_github_credentials() and get_gittea_credentials() were basically
doing the same thing, so i've merged them into one function.

I've also added toml parsing which by default will look in the same
directory as the klonol.exe for a file called "credentials.toml". If
this file is not found it will create it and populate it with the
following toml:
```toml
ACCESS_TOKEN="unset_value"
BASE_URL="github.com"
USERNAME="unset_value"
```

the default path for the toml file can be overriden with the command-line option "--credentials, -c".
Copy link
Owner

@hungrybluedev hungrybluedev left a comment

Choose a reason for hiding this comment

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

Look pretty good! Include the changes I suggested, and I'll let the CI run. I will try to use this locally on my machine to test my Gitea setup. If you want, I can make you an account on my instance for testing?

README.md Show resolved Hide resolved
credentials.v Outdated Show resolved Hide resolved
credentials.v Outdated Show resolved Hide resolved
@hungrybluedev hungrybluedev changed the title #1 toml config credentials: Use cross-platform credentials.toml instead of Unix-specific .env files. Feb 15, 2023
@hungrybluedev
Copy link
Owner

I've pushed a few commits to your fork, so remember to git pull before changing the code.

A1ex-N added 3 commits February 18, 2023 17:44
the function `get_value_or_set_default(creds toml.Doc, value string, default string)`
returns a value for a key if the key exists and is not empty, or returns a default value.
`value_opt() or {}` will only set a
default value for a key that doesn't exist.
@A1ex-N
Copy link
Author

A1ex-N commented Feb 18, 2023

Sorry i took so long to get around to this. I've been pretty lazy lately.

the stuff you mentioned should be fixed now, but i'll let you be the judge of that.

I was thinking the toml file should look like this

[github]
username="etc"
base_url="etc"
access_token="etc"

[gittea]
username="etc"
base_url="etc"
access_token="etc

because currently if you wanted gittea and github credentials saved you'll have to have something like this, and just comment out the credentials you don't want to use. It might be annoying if you're frequently running klonol on both github and gittea

USERNAME="etc"
BASE_URL="github.com"
ACCESS_TOKEN="etc"

#USERNAME="etc"
#BASE_URL="gittea.com"
#ACCESS_TOKEN="etc"

I've actually never used klonol before (outside of testing) so i don't know how people would normally use it. I guess you'd be the best judge of that @hungrybluedev

@hungrybluedev
Copy link
Owner

Good idea. Then we can also include the provider info the credentials file and have it be optional as a flag. Then we can choose the section accordingly. Can you make these changes?

@hungrybluedev
Copy link
Owner

Also, we should update the .sample file to be TOML instead of ENV.

@A1ex-N
Copy link
Author

A1ex-N commented Feb 18, 2023

Good idea. Then we can also include the provider info the credentials file and have it be optional as a flag. Then we can choose the section accordingly. Can you make these changes?

I could do it, but it might be a while before i get around to doing it. I'd understand if you just want to take over and get it done. Sometimes i just don't feel like coding.

@hungrybluedev
Copy link
Owner

It's alright, there's no rush. I have to finish my assignments over the weekend, anyway. You can take your time with this.

I'll hop in once I get some work done.

Don't need this sample any more.
The -fast flag uses TCC for faster prototyping; using no flag outputs an optimised executable
Also update the Credential and Credential File structs.
Fix typos, add cross-platform feature, introduce TOML usage, update output, add acknowledgement.
@hungrybluedev hungrybluedev marked this pull request as ready for review March 5, 2023 11:27
@hungrybluedev hungrybluedev merged commit 7001e6c into hungrybluedev:main Mar 5, 2023
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.

2 participants