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

Use array for default users #108

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Nov 23, 2017

This also avoids shelling out to whoami always, and only uses $USER
by default.

@carnager
Copy link
Owner

carnager commented Nov 23, 2017

hmmm, Maybe instead of checking for old config we should update config instead?
Something like this:

readarray oldusers < <(grep default_user .config/rofi-pass/config)
default_users=(${a[@]##*=})
sed '/default_user/d' .config/rofi-pass/config
echo "default_users=(${array[@]})" > foo2

@blueyed
Copy link
Contributor Author

blueyed commented Nov 23, 2017

Maybe instead of checking for old config we should update config instead?

Might not be as easy, e.g. you have to ignore comments etc. - not sure if you do this in the 2nd line already (?!).

@blueyed
Copy link
Contributor Author

blueyed commented Nov 23, 2017

It is only barely tested btw, so please review with caution.

@carnager
Copy link
Owner

carnager commented Nov 23, 2017

[carnager@caprica ~]$ grep default_user .config/rofi-pass/config 
[email protected]
default_user2=rasi
#default_user3=carnager
[carnager@caprica ~]$ readarray oldusers < <(grep -P '^default_user|^default_user2' .config/rofi-pass/config)
[carnager@caprica ~]$ default_users=(${oldusers[@]##*=})
[carnager@caprica ~]$ echo "default_users=(${default_users[@]})"
default_users=([email protected] rasi)

Updating seems to work fine. Now have to test if this works in general

What I like about this aproach that one can have as many users as he/she likes.

@blueyed
Copy link
Contributor Author

blueyed commented Nov 23, 2017

Your approach would cause an extra grep run - while the config has been sourced already.
You also do not keep the order and/or respect the system config.

@carnager
Copy link
Owner

carnager commented Nov 3, 2018

@blueyed Do you think you can update this to latest master? Working through these old PRs and this is one I am interested to test and eventually merge.

@blueyed
Copy link
Contributor Author

blueyed commented Nov 4, 2018

@carnager
Done.
Not tested, and not much fun due to the spaces to tab changes.

@blueyed blueyed force-pushed the usernames branch 3 times, most recently from 982a022 to e0b8a62 Compare November 4, 2018 01:21
This also avoids shelling out to `whoami` always, and only uses `$USER`
by default.
@blueyed
Copy link
Contributor Author

blueyed commented Mar 2, 2019

Rebased again (no conflicts).

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