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.

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

Jump to bottom

Smaller improvements, fixes #5 #4

Merged
merged 16 commits into from
Jan 2, 2021
Merged

Smaller improvements, fixes #5 #4

merged 16 commits into from
Jan 2, 2021

Conversation

AlexBocken
Copy link
Contributor

@AlexBocken AlexBocken commented Dec 30, 2020 •
edited
Loading

The filename expansion doesn't work for me since the newest merge. Might be because I'm using
dash as my sh instead of the default of bash as it is for most distros.

All in all just smaller improvements, I think the commit messages should be pretty self-explanatory. I think the only issue that could arise might be the chmod +x i did. Lemme know if i should remove that last commit.

3bdb2e1 fixes the issue mentioned in #3 (comment) ( #5 )

@AlexBocken
Copy link
Contributor Author

added two more commits to clean up stuff a bit more.

@saloniamatteo
Copy link
Contributor

Looks great!

I didn't know about the bug where sxiv wouldn't open the images because I wrote the script on my phone, but looks like you fixed it so hopefully @Bugswriter merges this pull request

@saloniamatteo
Add more comments & more variables
b4bbfb9 
Added more comments to clarify what the program is doing, as well as add more variables, to make the bash script more flexible, allowing users to only change one variable instead of manually changing all instances of the hardcoded path.
@saloniamatteo
Update README.md
3ba1b21
@AlexBocken AlexBocken changed the title Smaller improvements Smaller improvements, fixes #5 Dec 30, 2020
@AlexBocken
Copy link
Contributor Author

AlexBocken commented Dec 30, 2020 •
edited
Loading

Implemented in commit b699a1f advice of @sayan01 in c765563#r45525422

@saloniamatteo saloniamatteo mentioned this pull request Dec 30, 2020
Closed
DusanLesan
DusanLesan reviewed Dec 30, 2020
View reviewed changes
README.md Outdated Show resolved Hide resolved
@saloniamatteo
Copy link
Contributor

@AlexBocken please check my latest PR, you have accidentally pointed the config directory to "/tmp/redyt" instead of pointing the cache directory there.

@AlexBocken
Copy link
Contributor Author

AlexBocken commented Dec 30, 2020 •
edited
Loading

@AlexBocken please check my latest PR, you have accidentally pointed the config directory to "/tmp/redyt" instead of pointing the cache directory there.

Good catch! sorry about that. fixed it.

Edit: bruh, I think that's enough for today. typos after typos from my side.

@saloniamatteo
Copy link
Contributor

bruh, I think that's enough for today. typos after typos from my side.

Heh, don't worry.

@Bugswriter
Copy link
Owner

Bugswriter commented Jan 1, 2021 •
edited
Loading

Sorry for being late. I want to merge this but .. It's having some merge conflicts. I will take some time. I have my exams going so .. pretty busy these days.

@AlexBocken
Copy link
Contributor Author

It's having some merge conflicts.

Ah sorry I think that was an issue from my side. It should work now, right?
Anyways, take your time with github stuff during your exam period. Best of luck with those btw!

@Bugswriter Bugswriter merged commit 2907947 into Bugswriter:main Jan 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JordyUSA JordyUSA JordyUSA approved these changes

@saloniamatteo saloniamatteo saloniamatteo approved these changes

@DusanLesan DusanLesan DusanLesan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@AlexBocken @saloniamatteo @Bugswriter @DusanLesan @JordyUSA