-
Notifications
You must be signed in to change notification settings - Fork 17
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
Work on Show PKGBUILD diff when updating a package issue #62
base: main
Are you sure you want to change the base?
Conversation
Need to test it, more work on pbrisbin#41
Hey there- I'm a bot, here to let you know that some code in this PR might not NOTE: Since this PR was opened from a fork, we're not able to open our PR If you're interested in incorporating the style fixes in this PR, you can do git remote add upstream https://github.com/pbrisbin/aurget.git
git fetch upstream pull/63/head
git merge --ff-only FETCH_HEAD
git push Fixing the styling (through the above or any other means) will cause the linked Sorry if this was unexpected. To disable it, see our documentation. |
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.
Thanks for getting started on this!
aurget
Outdated
@@ -331,7 +342,7 @@ set_defaults() { | |||
silent=false | |||
sort_mode='name' | |||
sync_mode='install' | |||
temp_directory='/tmp/aurget' | |||
pkgbuild_directory='~/.cache/aurget' |
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 would call this
cache_directory
- It should follow XDG,
cache_directory=${XDG_CACHE_HOME:-$HOME/.cache}/aurget
aurget
Outdated
@@ -554,7 +565,7 @@ setup_targets() { | |||
version="${versions[$name]}" | |||
|
|||
if [[ -z "$version" ]]; then | |||
pkgbuild="$temp_directory/${name}_PKGBUILD" | |||
pkgbuild="$pkgbuild_directory/${name}_PKGBUILD" |
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 would place the file at $cache/$name/$version/PKGBUILD
-- You're going to need $version
in the filename to find it later for diff
ing. But... you don't actually know the version
yet, so you might still have to download it to some temporary location and move it after processing. I'm open to ideas here.
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.
That was my thought as well, maybe there is a way to hold it in a folder and parse it and then moving it into it's own directory in name/version before continuing?
aurget
Outdated
|
||
if clean; then | ||
clean_package_directory | ||
fi | ||
if searching; then |
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.
Do you intend for the behavior of --clean
to be that you clean and can proceed with other operations, or do you want --clean
to be something that has to be used by itself?
If it's the former, that shouldn't be represented by opmode=clean
it should a boolean variable like clean=true
. Then you can just do clean && clean_...
here.
If it's the latter, this should be if; clean; then; elif searching; then; else
-- or maybe a case
?
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.
Which would be more preferable to you?
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 don't have a a strong opinion about it. I'd probably do the former if it were up to me, but that's a bigger change from the code you have here now.
then | ||
else | ||
mkdir "$pkgbuild_directory" || die 'unable to create package directory' | ||
fi |
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.
This is not formatted correctly. Please use the other if
expressions in the file as examples.
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.
Sorry, I will take a look.
Pull request conversation: pbrisbin#62 main issue: pbrisbin#41
Hey, sorry, I made an issue I should have noticed. $cache should be should cache_directory. |
@LilyCathelineau are you still interested in finishing this pull request? Otherwise I would look into that. |
#41