-
Notifications
You must be signed in to change notification settings - Fork 45
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
Feature/brave cache #76
base: master
Are you sure you want to change the base?
Conversation
} | ||
|
||
# Check what to do with brave | ||
check_brave_cache() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR.
What if you simplify adding this browser by joining all the separate functions you did into one as it is done for other browsers, i.e chrome?
The ln
behaviour should not be changing, it will break many things for them I suspect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I broke it up into several functions because I needed to do several more than once to prepare for previously-enabled cache. If you want me to do it I can make it a single function and... copy and paste some code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And speaking of the ln
behaviour: it cost me a lot of time to figure out the problem. I was not expecting it as system tools usually behave as expected
close_app "Brave Browser" | ||
make_brave_dir | ||
move_brave_cache | ||
link_brave_dir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider replacing this line with line 225:
/bin/ln -v -s "${USERRAMDISK}"/Brave-Browser ~/Library/Caches/BraveSoftware/Brave-Browser
it is used once only, does not needs to be a func
if user_response "${MSG_PROMPT_FOUND}" 'Brave'"${MSG_MOVE_CACHE}" ; then | ||
close_app "Brave Browser" | ||
make_brave_dir | ||
move_brave_cache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
body of the function could be just added here.
@jdarowski if you could address these 2 comments, I'm happy to merge the PR, LGTM otherwise. |
What it does?
I have tested it on
NOTICE
I based my solution on Chromium's cache moving and I noticed that the
-f
and-F
forln
don't seem to have any effect on Catalina. All caches that use these flags don't work on Catalina for me, so I made a workaround for Brave. I'd be happy to do the same workaround for other browsers, but I didn't want to mix things together