elebumm / RedditVideoMakerBot

Create Reddit Videos with just✨ one command ✨
GNU General Public License v3.0
6.59k stars 1.84k forks source link

NEED HELP, the env folder looks right im not sure why it wont work #250

Closed jordan1121 closed 2 years ago

jordan1121 commented 2 years ago

same issue Capture

jordan1121 commented 2 years ago

I already tried using askreddit in subreddit

TheRealPerson98 commented 2 years ago

thats the outdated env please download the update project and try

jordan1121 commented 2 years ago

same thing Same issue

callumio commented 2 years ago

Can you make sure your .env file is just .env and not .env.txt or anything like that.

If you type dir into the terminal whilst in that folder, and send the output I can help you more 😄

itszechs commented 2 years ago

To avoid such problems in future, we could store the config in $HOME directory inside .config folder which can be accessed using

>>> from pathlib import Path
>>> Path.home()
PosixPath('/home/username')

No, since there is already a ongoing PR to improve the setup process, it could just ask users for creds and save it in $HOME folder.

Improve setup: PR https://github.com/elebumm/RedditVideoMakerBot/pull/125

blockarchitech commented 2 years ago

To avoid such problems in future, we could store the config in $HOME directory inside .config folder which can be accessed using

This won't work, since windows doesn't have a $HOME dir. We could use os to find the OS, but that might complicate things.

callumio commented 2 years ago

To add on to that, it's also just not necessary, then env file works perfectly fine

itszechs commented 2 years ago

windows doesn't have a $HOME dir

we are not actually looking for $HOME environment variable Path.home() returns the HOME dir path on all systems.

Example on windows (prior example was from Linux):

>>> from pathlib import Path
>>> Path.home()
WindowsPath('C:/Users/username')
callumio commented 2 years ago

But what benefit does that actually bring?

blockarchitech commented 2 years ago

we are not actually looking for $HOME environment variable Path.home() returns the HOME dir path on all systems.

MANY windows systems do not give this output. They will give a decorator like %SHELL% for example, forcing you (yet again) to use os. Also, there really isn't any point since .env works fine. I guess you could store it in appdata if you really want to but there's no point.

not to mention, on many systems pathlib doesn't really work, plus its incredibly slow compared to python's built-in os. There's no point in adding a new, slow import when one that's fast (and not to mention asynchronous) library already exists.

Plus, if WindowsPath('C:/Users/username') is the output that's given, we can't really use that at all.

itszechs commented 2 years ago

MANY windows systems do not give this output. They will give a decorator like %SHELL%

I was not aware of that, that's a interesting. Do you have any issue/stackoverflow post of that happening?

forcing you (yet again) to use os

you mean like this...

>>> import os
>>> os.path.expanduser('~')
'/home/username'

Sure, Path.home() does the exact same thing.

could store it in appdata if you really want to but there's no point.

It's usually a good practice to store settings and configuration is user's home directory /home/username/.config/appname/config.conf

Plus, if WindowsPath('C:/Users/username') is the output that's given, we can't really use that at all.

why do you say that? that Path object can be converted to string simply by str() or "dunder string" __str__() if we want something fast.

blockarchitech commented 2 years ago

I was not aware of that, that's a interesting. Do you have any issue/stackoverflow post of that happening?

This is based on personal experience. I'm sure you can find a document on this as well.

It's usually a good practice to store settings and configuration is user's home directory /home/username/.config/appname/config.conf

Once/if this is packaged and thrown on pipy, then we'd probably follow this. In this case scenario, it really has zero point unless you plan on uninstalling and re installing the program many times, which isn't the use case of this program.

why do you say that? that Path object can be converted to string simply by str() or "dunder string" str() if we want something fast.

We could use __str__(), however having a whole class dedicated to turning WindowsPath('C:/Users/username') into 'C:/Usersusername' is not really best coding practice.

I'm sure pathlib would be used in the future of this repo, but I really don't think it's useful and/or needed at this time.

callumio commented 2 years ago

Closing due to being stale, config is being reworked