Skip to content

PR for Issues #7 #8

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

PR for Issues #7 #8

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5ebe11f
PR for Issues #7
TheRaccoon123 Jan 9, 2022
a0d1b67
Merge branch 'master' into patch-1
beliaev-maksim Jan 12, 2022
5378d9f
Where will this go?
TheRaccoon123 Jan 12, 2022
56b60fc
This might work
TheRaccoon123 Jan 12, 2022
10581ea
update This might work
TheRaccoon123 Jan 13, 2022
afaf54c
The can work ?
TheRaccoon123 Jan 13, 2022
0943780
appropriate fixes
TheRaccoon123 Jan 14, 2022
2d07732
Read me format
TheRaccoon123 Jan 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ setenv -l

## How to use as Python package
### Get variable
Get value of variable _my_var_
Get value of variable _my_var_
Note Throws error KeyError when Enviroment variable is not found
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
Note Throws error KeyError when Enviroment variable is not found
> Note: Throws error `KeyError` when Environment variable is not found
>

```python
from py_setenv import setenv
setenv("my_var")
Expand Down
9 changes: 5 additions & 4 deletions py_setenv.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def setenv(name="", value=None, user=False, append=False, delete=False, list_all
elif delete:
result = delete_variable(name, user)
else:
result = get_variable(name, user)
result = get_variable(name, user, suppress_echo)

if not suppress_echo:
click.echo(result)
Expand Down Expand Up @@ -97,7 +97,7 @@ def append_variable(name, value, user):
return result


def get_variable(name, user):
def get_variable(name, user, suppress_echo):
"""
Gets the value of environment variable
"""
Expand All @@ -107,8 +107,9 @@ def get_variable(name, user):
value, regtype = winreg.QueryValueEx(key, name)
return value
except WindowsError:
click.echo("Environment Variable '{}' does not exist".format(name))
return ""
if not suppress_echo:
click.echo("Environment Variable '{}' does not exist".format(name))
raise KeyError
Copy link
Owner

Choose a reason for hiding this comment

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

I have some concerns how this will work in CLI implementation. It will be a long unreadable traceback.

I think we need to raise a KeyError for Python module and just return empty and error message in CLI

Copy link
Owner

Choose a reason for hiding this comment

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

also unittests fail with current implementation. Need to fix it as well


def delete_variable(name, user):
"""
Expand Down