[pacman-dev] [PATCH] pacdiff - merge just the newest .pacsave
Merge just the newest .pacsave, warn about any .pacsave.N files at the end
Signed-off-by: Jonathan Frazier eyesw...@gmail.com
---
contrib/pacdiff.sh.in | 11 +++
1 file changed, 11 insertions(+)
diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in
index ee80c9c..a24a7c9 100644
--- a/contrib/pacdiff.sh.in
+++ b/contrib/pacdiff.sh.in
@@ -107,12 +107,21 @@ cmd() {
fi
}
+declare -a oldsaves
+
# see http://mywiki.wooledge.org/BashFAQ/020
while IFS= read -u 3 -r -d '' pacfile; do
file=${pacfile%.pac*}
file_type=pac${pacfile##*.pac}
+ # add matches for pacsave.N to oldsaves array, do not prompt
+ if [ ${file_type%%[[:digit:]]*} == pacsave. ]; then
+ oldsaves+=($pacfile)
+ continue
+ fi
+
msg %s file found for %s $file_type $file
+
if [ ! -f $file ]; then
warning $file does not exist
rm -iv $pacfile
@@ -139,6 +148,8 @@ while IFS= read -u 3 -r -d '' pacfile; do
fi
done 3 (cmd)
+(( ${#oldsaves[@]} 0 )) warning Ignoring %s ${oldsaves[@]}
+
exit 0
# vim: set ts=2 sw=2 noet:
--
1.8.3.1
Re: [pacman-dev] [PATCH] pacdiff using pacman db v2
Subject: [pacman-dev] [PATCH] pacdiff using pacman db v2
Please move that v2 into the PATCH tag like so: [PATCH v2]. Otherwise it
sounds like you use version 2 of the pacman db and it might end up in
the final commit.
I'd also suggest to change the subject to something like pacdiff: use
pacman db for lookups to increase performance so it's more obvious what
this patch does and why. Still missing the why is using that
faster/better than locate, but that can be in the commit message.
On 01.07.2013 02:00, Jonathan Frazier wrote:
Implement the default search using the local pacman db.
I'm not sure the default should switch to an inferior method (inferior
because it doesn't find everything). updatedb might be too slow on
certain hardware or whatever, but I think that's a minority so we should
stick with locate. Also that way long time users won't have left over
.pacsaves.
Use @sysconfdir@/pacman.conf to find the local db
Search for pacsave.0 files.
You also search for .pacsave, .pacsave.1, ..., .pacnew, .pacorig. No
need to mention this explicitly here. Also the comment about pacman.conf
doesn't belong here unless you explain why you do it this way and you
have a good reason not to do anything else.
Actually none of the 3 lines explain why any of this is done.
I know these are the changes you made compared to the previous patch,
but such changes belong to the comment section below (between the ---
marker and the list of changed files) not into the commit message.
All I remember (now) is that you do this because updatedb is slow on
some low end hardware, but that should really be mentioned here because
I might forget (or am I already wrong? I honestly don't know) and others
won't even have known in the first place and someone might care when
they look through the commit log in a few years to find why a certain
feature appeared (been there, done that).
Signed-off-by: Jonathan Frazier eyesw...@gmail.com
---
contrib/pacdiff.sh.in | 86
++-
1 file changed, 64 insertions(+), 22 deletions(-)
diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in
index 47779d6..ee80c9c 100644
--- a/contrib/pacdiff.sh.in
+++ b/contrib/pacdiff.sh.in
@@ -21,9 +21,8 @@
declare -r myname='pacdiff'
declare -r myver='@PACKAGE_VERSION@'
-diffprog=${DIFFPROG:-vimdiff}
-diffsearchpath=${DIFFSEARCHPATH:-/etc}
-locate=0
+diffprog=${DIFF_PROG:-vimdiff}
+difffindpath=${DIFF_FIND_PATH:-/etc}
Out of scope of this patch.
USE_COLOR='y'
m4_include(../scripts/library/output_format.sh)
@@ -32,11 +31,19 @@ m4_include(../scripts/library/term_colors.sh)
usage() {
echo $myname : a simple pacnew/pacorig/pacsave updater
- echo Usage : $myname [-l]
- echo -l/--locate makes $myname use locate rather than find
- echo DIFFPROG variable allows to override the default vimdiff
- echo DIFFSEARCHPATH allows to override the default /etc path
- echo Example : DIFFPROG=meld DIFFSEARCHPATH=\/boot /etc /usr\
$myname
+ echo
+ echo Usage : $myname [-lf]
+ echo -l/--locate makes $myname search using locate
+ echo -f/--find makes $myname search using find
Add an option for your pacsave method. Maybe -p.
+ echo
+ echo Note: the default search looks for backup files in the local
pacman db
+ echo this generally will not find *.pacsave files
+ echo
+ echo DIFF_PROG variable will override the default editor: vimdiff
+ echo DIFF_FIND_PATH will override the default path when using --find
+ echo
+ echo Example : DIFF_PROG=meld $myname
+ echo Example : DIFF_FIND_PATH=\/boot /etc /usr\ $myname -f
Out of scope.
}
version() {
@@ -45,27 +52,61 @@ version() {
echo 'Copyright (C) 2013 Pacman Development Team
pacman-dev@archlinux.org'
}
-cmd() {
- if [ $locate -eq 1 ]; then
- locate -0 -e -b \*.pacnew \*.pacorig \*.pacsave
- else
- find $diffsearchpath \( -name \*.pacnew -o -name \*.pacorig -o
-name \*.pacsave \) -print0
- fi
-}
-
Any reason you moved cmd()? Makes reviewing the changes harder.
if [ $# -gt 0 ]; then
case $1 in
-l|--locate)
- locate=1;;
+ locate=$(type -P locate);;
+ -f|--find)
+ find=$(type -P find);;
Why did you change that to do a path lookup and not exit if you can't
find it?
-V|--version)
- version; exit 0;;
+ version; exit 0;;
-h|--help)
- usage; exit 0;;
+ usage; exit 0;;
*)
- usage; exit 1;;
+ usage; exit 1;;
esac
fi
+if [[ ! -n $locate ! -n $find ]]; then
Don't do that. Someone might add a 4th option in the future and then
they have to adjust everything again.
Just use a $mode variable and set it to a default or
[pacman-dev] (no subject)
Subject: re: [pacman-dev] [PATCH] pacdiff using pacman db v2
Reply-To: In-Reply-To: 51d1468a.3030...@xinu.at
On 07/01/13 at 11:06am, Florian Pritz wrote:
Subject: [pacman-dev] [PATCH] pacdiff using pacman db v2
Please move that v2 into the PATCH tag like so: [PATCH v2]. Otherwise
it sounds like you use version 2 of the pacman db and it might end
up in the final commit.
Ok will do.
I'd also suggest to change the subject to something like pacdiff: use
pacman db for lookups to increase performance so it's more obvious
what this patch does and why. Still missing the why is using that
faster/better than locate, but that can be in the commit message.
On 01.07.2013 02:00, Jonathan Frazier wrote:
Implement the default search using the local pacman db.
I'm not sure the default should switch to an inferior method (inferior
because it doesn't find everything). updatedb might be too slow on
certain hardware or whatever, but I think that's a minority so we
should stick with locate. Also that way long time users won't have
left over .pacsaves.
First let me say that I am not stuck on this being the default option,
but it is the one I will use. I could say locate is inferior because it
finds pacsaves not in use. giving more noise and less signal. My
rational is more that it finds the active configs more throughly. Speed
is not the only factor. I will try and add more descriptive commit msgs.
Use @sysconfdir@/pacman.conf to find the local db Search for
pacsave.0 files.
You also search for .pacsave, .pacsave.1, ..., .pacnew, .pacorig. No
need to mention this explicitly here. Also the comment about
pacman.conf doesn't belong here unless you explain why you do it this
way and you have a good reason not to do anything else.
Actually none of the 3 lines explain why any of this is done.
I know these are the changes you made compared to the previous patch,
but such changes belong to the comment section below (between the
--- marker and the list of changed files) not into the commit
message.
Adding support for these extra pacsave files is a new feature and worth
noting in the changelog. this feature was requested in another thread. I
can make it a separate patch if the rest of this is going to take longer.
All I remember (now) is that you do this because updatedb is slow on
some low end hardware, but that should really be mentioned here
because I might forget (or am I already wrong? I honestly don't know)
and others won't even have known in the first place and someone might
care when they look through the commit log in a few years to find why
a certain feature appeared (been there, done that).
I will try to be more descriptive in my changlog.
Signed-off-by: Jonathan Frazier eyesw...@gmail.com ---
contrib/pacdiff.sh.in | 86
++- 1 file changed,
64 insertions(+), 22 deletions(-)
diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index
47779d6..ee80c9c 100644 --- a/contrib/pacdiff.sh.in +++
b/contrib/pacdiff.sh.in @@ -21,9 +21,8 @@ declare -r
myname='pacdiff' declare -r myver='@PACKAGE_VERSION@'
-diffprog=${DIFFPROG:-vimdiff}
-diffsearchpath=${DIFFSEARCHPATH:-/etc} -locate=0
+diffprog=${DIFF_PROG:-vimdiff}
+difffindpath=${DIFF_FIND_PATH:-/etc}
Out of scope of this patch.
DIFFSEARCHPATH is misleading and I find it difficult to read. the old name
implies it works for all search types, when it only applied to find.
Since nobody is defending find as the default, it should be changed
along with the default. Adding underscores was just cause I am fixing
it. Is DIFFFINDPATH more acceptable?
USE_COLOR='y'
m4_include(../scripts/library/output_format.sh) @@ -32,11 +31,19 @@
m4_include(../scripts/library/term_colors.sh)
usage() { echo $myname : a simple pacnew/pacorig/pacsave updater
- echo Usage : $myname [-l] - echo -l/--locate makes $myname
use locate rather than find - echo DIFFPROG variable allows to
override the default vimdiff -echo DIFFSEARCHPATH allows to
override the default /etc path - echo Example : DIFFPROG=meld
DIFFSEARCHPATH=\/boot /etc /usr\ $myname + echo + echo
Usage : $myname [-lf] + echo -l/--locate makes $myname search
using locate +echo -f/--find makes $myname search using
find
Add an option for your pacsave method. Maybe -p.
Does it make sense to have an option for the default? I don't think so.
Whatever the default ends up being it doesn't need a option.
+ echo + echo Note: the default search looks for backup files in
the local pacman db + echo this generally will not find
*.pacsave files + echo + echo DIFF_PROG variable will
override the default editor: vimdiff + echo DIFF_FIND_PATH will
override the default path when using --find + echo + echo
Example : DIFF_PROG=meld $myname +echo Example :
