[pacman-dev] [PATCH] repo-add: reuse write_list_entry for desc file

[Dave Reisner]

On Mon, Jun 20, 2011 at 01:27:07PM -0500, Dan McGee wrote:
> On Mon, Jun 20, 2011 at 1:12 PM, Dave Reisner <d at falconindy.com> wrote:
> > Repurpose the write_list_entry function for everything we write to the
> > depends and desc files by appending our own extra newline after each
> > single line data point.
> >
> > This also refactors write_list_entry into a single printf statement.
> >
> > Signed-off-by: Dave Reisner <d at falconindy.com>
> > ---
> > More repo-add madness. Reusable code, ftw.
> >
> > dave
> >
> >  scripts/repo-add.sh.in |   31 +++++++++++++++----------------
> >  1 files changed, 15 insertions(+), 16 deletions(-)
> >
> > diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
> > index 14506dc..a23f89e 100644
> > --- a/scripts/repo-add.sh.in
> > +++ b/scripts/repo-add.sh.in
> > @@ -88,8 +88,7 @@ There is NO WARRANTY, to the extent permitted by law.\n")"
> >  #              arg3 - File to write to
> >  write_list_entry() {
> >        if [[ -n $2 ]]; then
> > -               echo "%$1%" >>$3
> > -               echo -e $2 >>$3
> > +               printf "%%%s%%\n$2\n" "$1" >>"$3"
> Does it make sense to use both %s and var substitution at the same
> time? Why not:
> printf "%%%s%%\n%s\n" "$1" "$2" >>"$3"
> 

$ foo=bar\nbaz
$ print '%s\n' "$foo"
bar\nbaz

In other words, the embedded newlines have to be within the format.
But really, I'm just going to scrap this and redo it properly. We should
use arrays here and a proper format function that looks something like:

format_list_entry() {
  local field=$1; shift

  if (( $# )); then
    printf '%%%s%%\n' "$field"
    printf '%s\n' "$@"
    printf '\n'
  fi
}

We then do the writing to file on the calling side, e.g.,

{
  format_list_entry 'PKGNAME' foo
  format_list_entry 'PKGVER' 1.2.3
  format_list_entry 'DEPENDS' 'libfoo' 'libbar' 'libbaz'
} >desc

> >        fi
> >  }
> >
> > @@ -301,27 +300,27 @@ db_write_entry()
> >
> >        # create desc entry
> >        msg2 "$(gettext "Creating '%s' db entry...")" 'desc'
> > -       echo -e "%FILENAME%\n${1##*/}\n" >>desc
> > -       echo -e "%NAME%\n$pkgname\n" >>desc
> > -       [[ -n $pkgbase ]] && echo -e "%BASE%\n$pkgbase\n" >>desc
> > -       echo -e "%VERSION%\n$pkgver\n" >>desc
> > -       [[ -n $pkgdesc ]] && echo -e "%DESC%\n$pkgdesc\n" >>desc
> > +       write_list_entry "FILENAME" "${1##*/}\n" "desc"
> > +       write_list_entry "NAME" "$pkgname\n" "desc"
> > +       [[ -n $pkgbase ]] && write_list_entry "BASE" "$pkgbase\n" "desc"
> It would be cleaner to move the -n bits inside write_list_entry.
> Otherwise I like this change to make things a bit more consistent.
> 

Agreed. Makes things a lot cleaner and easier to read. Covered above in
conceptual blah blah blah.

> > +       write_list_entry "VERSION" "$pkgver\n" "desc"
> > +       [[ -n $pkgdesc ]] && write_list_entry "DESC" "$pkgdesc\n" "desc"
> >        write_list_entry "GROUPS" "$_groups" "desc"
> > -       [[ -n $csize ]] && echo -e "%CSIZE%\n$csize\n" >>desc
> > -       [[ -n $size ]] && echo -e "%ISIZE%\n$size\n" >>desc
> > +       [[ -n $csize ]] && write_list_entry "CSIZE" "$csize\n" "desc"
> > +       [[ -n $size ]] && write_list_entry "ISIZE" "$size\n" "desc"
> >
> >        # add checksums
> > -       echo -e "%MD5SUM%\n$md5sum\n" >>desc
> > -       echo -e "%SHA256SUM%\n$sha256sum\n" >>desc
> > +       write_list_entry "MD5SUM" "$md5sum\n" "desc"
> > +       write_list_entry "SHA256SUM" "$sha256sum\n" "desc"
> >
> >        # add PGP sig
> > -       [[ -n $pgpsig ]] && echo -e "%PGPSIG%\n$pgpsig\n" >>desc
> > +       [[ -n $pgpsig ]] && write_list_entry "PGPSIG" "$pgpsig\n" "desc"
> >
> > -       [[ -n $url ]] && echo -e "%URL%\n$url\n" >>desc
> > +       [[ -n $url ]] && write_list_entry "URL" "$url\n" "desc"
> >        write_list_entry "LICENSE" "$_licenses" "desc"
> > -       [[ -n $arch ]] && echo -e "%ARCH%\n$arch\n" >>desc
> > -       [[ -n $builddate ]] && echo -e "%BUILDDATE%\n$builddate\n" >>desc
> > -       [[ -n $packager ]] && echo -e "%PACKAGER%\n$packager\n" >>desc
> > +       [[ -n $arch ]] && write_list_entry "ARCH" "$arch\n" "desc"
> > +       [[ -n $builddate ]] && write_list_entry "BUILDDATE" "$builddate\n" "desc"
> > +       [[ -n $packager ]] && write_list_entry "PACKAGER" "$packager\n" "desc"
> >        write_list_entry "REPLACES" "$_replaces" "desc"
> >
> >        # create depends entry
> > --
> > 1.7.5.4
> >
> >
> >
>