Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Gentoo: Dev

enhancement for doicon/newicon in eutils.eclass

 

 

Gentoo dev RSS feed   Index | Next | Previous | View Threaded


hasufell at gentoo

May 20, 2012, 4:24 PM

Post #1 of 22 (389 views)
Permalink
enhancement for doicon/newicon in eutils.eclass

I want support for installing icons into the appropriate directories
which are under /usr/share/icons/... and not just pixmaps.

proposal attached + diff

This should not break existing ebuilds. Tested a bit and open for review
now.
Attachments: eutils-doicon-newicon.eclass (3.05 KB)
  eutils.eclass.diff (3.74 KB)


aballier at gentoo

May 20, 2012, 4:36 PM

Post #2 of 22 (385 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On Mon, 21 May 2012 01:24:13 +0200
hasufell <hasufell [at] gentoo> wrote:

> I want support for installing icons into the appropriate directories
> which are under /usr/share/icons/... and not just pixmaps.
>
> proposal attached + diff
>
> This should not break existing ebuilds. Tested a bit and open for
> review now.

maybe i missed something but cant you just make doicon a newicon
wrapper and remove all that code duplication ?


hasufell at gentoo

May 20, 2012, 4:49 PM

Post #3 of 22 (383 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On 05/21/2012 01:36 AM, Alexis Ballier wrote:
> On Mon, 21 May 2012 01:24:13 +0200
> hasufell <hasufell [at] gentoo> wrote:
>
>> I want support for installing icons into the appropriate directories
>> which are under /usr/share/icons/... and not just pixmaps.
>>
>> proposal attached + diff
>>
>> This should not break existing ebuilds. Tested a bit and open for
>> review now.
>
> maybe i missed something but cant you just make doicon a newicon
> wrapper and remove all that code duplication ?
>

I don't see how. "doicon" supports installing multiple icons with one
command, as well as directories.
That does not work for "newicon".


abcd at gentoo

May 20, 2012, 5:01 PM

Post #4 of 22 (385 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 05/20/2012 07:49 PM, hasufell wrote:
> On 05/21/2012 01:36 AM, Alexis Ballier wrote:
>> On Mon, 21 May 2012 01:24:13 +0200 hasufell <hasufell [at] gentoo>
>> wrote:
>>
>>> I want support for installing icons into the appropriate
>>> directories which are under /usr/share/icons/... and not just
>>> pixmaps.
>>>
>>> proposal attached + diff
>>>
>>> This should not break existing ebuilds. Tested a bit and open
>>> for review now.
>>
>> maybe i missed something but cant you just make doicon a newicon
>> wrapper and remove all that code duplication ?
>>
>
> I don't see how. "doicon" supports installing multiple icons with
> one command, as well as directories. That does not work for
> "newicon".
>
>

Normally, new* is a wrapper for do* that does something like:

newfoo() {
# argument checking omitted...
cp -P "${1}" "${T}/${2}"
dofoo "${T}/${2}"
}

- --
Jonathan Callen
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBCgAGBQJPuYXpAAoJELHSF2kinlg4C7kP/1tNBut+o5s0ScBt/SREjrQr
Oy1Q4iRk1hV5NqY0ve4Cx1LqT3LKFWGCUkUY9pY4YoacG5Pbt4FKebqHqBjNASbg
FIwA2BNGZ5VKW2uwfqpTcc2lp6VRAfA3IxqkO3mlKy/zwk96G3jeoU+2sLSRatRn
ZiXsuCQ6DxrBRp8Jtjc5X9lIvbfBNoDIC7uXtW8fOuFBILYT/oSDPwUdT9r3ppNR
nfEuB3AsjPEyxuSd0R7QNYXexwDwVyp4JlpqFAQGH/+xUR4Nsy4Cw5jVwOr6Ip+3
afDVC4NNkFBUf+8qQ/Rd6Fdch3RVLeGtdIfhHbYgSbmUGFjIOWQ/+kEy6yp8jNlp
2nkA29Y9eEmRYSwwfQR4xzTcWnuPyK9cNKuK2L5LTUN143rHJdQ85lzkhNPIAYJO
jQVPWZh966tQrjkLB9sfW14mkCcvkK5q9F4re5Z44R1RHfdnl+Npe7uts17dQSCg
61sdTQ4Q7r81SyqSgsf2g+QaaLQ+d7+HrZvEJ+N8xhDOwX06CZVjvNWnwewbHBUT
jWHkilE9wKJcliaAwK6wqCMNw0LZnvQJHWUKm+vNKcl1PSg51VeQrjEldPDix5cS
WVCjsFjzo4pLzID19pI1z3Yx7NIJJTbqfKw1Oju+Bv3JGAzz12RCX8XV4t9BCp7r
tPk3xzb4gsxPDqROppVh
=D2sX
-----END PGP SIGNATURE-----


hasufell at gentoo

May 20, 2012, 10:30 PM

Post #5 of 22 (378 views)
Permalink
Re: Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On 05/21/2012 02:01 AM, Jonathan Callen wrote:
> On 05/20/2012 07:49 PM, hasufell wrote:
>> On 05/21/2012 01:36 AM, Alexis Ballier wrote:
>>> On Mon, 21 May 2012 01:24:13 +0200 hasufell
>>> <hasufell [at] gentoo> wrote:
>>>
>>>> I want support for installing icons into the appropriate
>>>> directories which are under /usr/share/icons/... and not
>>>> just pixmaps.
>>>>
>>>> proposal attached + diff
>>>>
>>>> This should not break existing ebuilds. Tested a bit and
>>>> open for review now.
>>>
>>> maybe i missed something but cant you just make doicon a
>>> newicon wrapper and remove all that code duplication ?
>>>
>
>> I don't see how. "doicon" supports installing multiple icons
>> with one command, as well as directories. That does not work for
>> "newicon".
>
>
>
> Normally, new* is a wrapper for do* that does something like:
>
> newfoo() { # argument checking omitted... cp -P "${1}" "${T}/${2}"
> dofoo "${T}/${2}" }
>
>

That does not use "newins" like the old function. Why would I want to
change that?


floppym at gentoo

May 21, 2012, 7:41 AM

Post #6 of 22 (380 views)
Permalink
Re: Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On Mon, May 21, 2012 at 1:30 AM, hasufell <hasufell [at] gentoo> wrote:
> On 05/21/2012 02:01 AM, Jonathan Callen wrote:
>> On 05/20/2012 07:49 PM, hasufell wrote:
>>> On 05/21/2012 01:36 AM, Alexis Ballier wrote:
>>>> On Mon, 21 May 2012 01:24:13 +0200 hasufell
>>>> <hasufell [at] gentoo> wrote:
>>>>
>>>>> I want support for installing icons into the appropriate
>>>>> directories which are under /usr/share/icons/... and not
>>>>> just pixmaps.
>>>>>
>>>>> proposal attached + diff
>>>>>
>>>>> This should not break existing ebuilds. Tested a bit and
>>>>> open for review now.
>>>>
>>>> maybe i missed something but cant you just make doicon a
>>>> newicon wrapper and remove all that code duplication ?
>>>>
>>
>>> I don't see how. "doicon" supports installing multiple icons
>>> with one command, as well as directories. That does not work for
>>> "newicon".
>>
>>
>>
>> Normally, new* is a wrapper for do* that does something like:
>>
>> newfoo() { # argument checking omitted... cp -P "${1}" "${T}/${2}"
>> dofoo "${T}/${2}" }
>>
>>
>
> That does not use "newins" like the old function. Why would I want to
> change that?
>

An alternative would be to factor the common code into a third
function, and call that from doicon and newicon.


mgorny at gentoo

May 21, 2012, 10:14 AM

Post #7 of 22 (375 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On Mon, 21 May 2012 01:24:13 +0200
hasufell <hasufell [at] gentoo> wrote:

> I want support for installing icons into the appropriate directories
> which are under /usr/share/icons/... and not just pixmaps.
>
> proposal attached + diff
>
> This should not break existing ebuilds. Tested a bit and open for
> review now.

I'd rather see a new function for that rather than making doicon()
overcomplex.

--
Best regards,
Michał Górny
Attachments: signature.asc (0.31 KB)


ssuominen at gentoo

May 21, 2012, 10:32 AM

Post #8 of 22 (377 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On 05/21/2012 08:14 PM, Michał Górny wrote:
> On Mon, 21 May 2012 01:24:13 +0200
> hasufell<hasufell [at] gentoo> wrote:
>
>> I want support for installing icons into the appropriate directories
>> which are under /usr/share/icons/... and not just pixmaps.
>>
>> proposal attached + diff
>>
>> This should not break existing ebuilds. Tested a bit and open for
>> review now.
>
> I'd rather see a new function for that rather than making doicon()
> overcomplex.
>

Uh, please no. Doing something simple as installing icons shouldn't have
multiple commands.

Should be easy enough to retain the old behavior, and add on top of that.

- Samuli


hasufell at gentoo

May 21, 2012, 12:58 PM

Post #9 of 22 (380 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On 05/21/2012 07:14 PM, Michał Górny wrote:
> On Mon, 21 May 2012 01:24:13 +0200 hasufell <hasufell [at] gentoo>
> wrote:
>
>> I want support for installing icons into the appropriate
>> directories which are under /usr/share/icons/... and not just
>> pixmaps.
>>
>> proposal attached + diff
>>
>> This should not break existing ebuilds. Tested a bit and open
>> for review now.
>
> I'd rather see a new function for that rather than making doicon()
> overcomplex.
>

The new features are totally optional, so there is nothing complex. If
someone is looking for support for /usr/share/icons/... directories
without setting insinto he can have it.

doicon/newicon without options (should) behave exactly the same way.


vapier at gentoo

May 21, 2012, 7:35 PM

Post #10 of 22 (378 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On Monday 21 May 2012 15:58:24 hasufell wrote:
> On 05/21/2012 07:14 PM, Michał Górny wrote:
> > On Mon, 21 May 2012 01:24:13 +0200 hasufell wrote:
> >> I want support for installing icons into the appropriate
> >> directories which are under /usr/share/icons/... and not just
> >> pixmaps.
> >>
> >> proposal attached + diff
> >>
> >> This should not break existing ebuilds. Tested a bit and open
> >> for review now.
> >
> > I'd rather see a new function for that rather than making doicon()
> > overcomplex.
>
> The new features are totally optional, so there is nothing complex. If
> someone is looking for support for /usr/share/icons/... directories
> without setting insinto he can have it.
>
> doicon/newicon without options (should) behave exactly the same way.

i think he's talking about the actual implementation details (and yes, the
duplication of content should be solved rather than copying & pasting the guts
-- someone suggested a good fix for that) and the extended option set (which i
think we can live with).
-mike
Attachments: signature.asc (0.82 KB)


vapier at gentoo

May 21, 2012, 7:49 PM

Post #11 of 22 (376 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On Sunday 20 May 2012 19:24:13 hasufell wrote:
> case ${2} in

please use $1/$2/etc... with positional variables when possible

> 16|22|24|32|36|48|64|72|96|128|192|256)
> size=${2}x${2};;
> 16x16|22x22|24x24|32x32|36x36|48x48|64x64|72x72|96x96|128x128|
192x192|256x256)
> size=${2};;
> scalable)
> size=scalable;;
> *)
> eqawarn "${2} is an unsupported icon size!"
> ((++ret));;
> esac

you can write this w/out having to duplicate two lists:
size=
if [[ $2 == "scalable" ]] ; then
size=$2
elif [[ ${2:0:2}x${2:0:2} == "$2" ]] ; then
size=${2:0:2}
case ${size} in
16|22|24|32|36|48|64|72|96|128|192|256) ;;
*) size= ;;
esac
fi
if [[ -z ${size} ]] ; then
eqawarn "${2} is an unsupported icon size!"
((++ret))
fi
shift 2

shift 2;;
-t|--theme)
theme=${2}
shift 2;;
-c|--context)
context=${2}
shift 2;;
*)
> if [[ -z ${size} ]] ; then
> dir=/usr/share/pixmaps
> else
> dir=/usr/share/icons/${theme}/${size}/${context}
> fi
>
> insinto "${dir}"

considering you only use $dir once, you could just call `insinto` directly on
the path rather than using the dir variable at all

> elif [[ -d ${1} ]] ; then
> for i in "${1}"/*.{png,svg} ; do
> doins "${i}"
> ((ret+=$?))
> done

why loop ? `doins "${1}"/*.{png,svg}` works just as well

you probably want to enable nullglobbing here, otherwise this will cause
problems if you try to doicon on a dir that contains just svg.

also, what about other file types ? people install xpm, svgz, gif, and other
file types ...

> exit ${ret}

bash masks error codes to [0..255], so all the ret updates should probably be
changed to just: ret=1

after all, i doubt anyone cares how many errors there were, just that one
occurred. and while you're here, might want to make it auto die on failure
like we've done with all our other helpers.
-mike
Attachments: signature.asc (0.82 KB)


hasufell at gentoo

May 23, 2012, 5:16 PM

Post #12 of 22 (376 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/22/2012 04:49 AM, Mike Frysinger wrote:
> On Sunday 20 May 2012 19:24:13 hasufell wrote:
>> case ${2} in
>
> please use $1/$2/etc... with positional variables when possible
>
>> 16|22|24|32|36|48|64|72|96|128|192|256) size=${2}x${2};;
>> 16x16|22x22|24x24|32x32|36x36|48x48|64x64|72x72|96x96|128x128|
> 192x192|256x256)
>> size=${2};; scalable) size=scalable;; *) eqawarn "${2} is an
>> unsupported icon size!" ((++ret));; esac
>
> you can write this w/out having to duplicate two lists: size= if [[
> $2 == "scalable" ]] ; then size=$2 elif [[ ${2:0:2}x${2:0:2} ==
> "$2" ]] ; then size=${2:0:2} case ${size} in
> 16|22|24|32|36|48|64|72|96|128|192|256) ;; *) size= ;; esac fi if
> [[ -z ${size} ]] ; then eqawarn "${2} is an unsupported icon
> size!" ((++ret)) fi shift 2
>
> shift 2;; -t|--theme) theme=${2} shift 2;; -c|--context)
> context=${2} shift 2;; *)
>> if [[ -z ${size} ]] ; then dir=/usr/share/pixmaps else
>> dir=/usr/share/icons/${theme}/${size}/${context} fi insinto
>> "${dir}"
>
> considering you only use $dir once, you could just call `insinto`
> directly on the path rather than using the dir variable at all
>
>> elif [[ -d ${1} ]] ; then for i in "${1}"/*.{png,svg} ; do doins
>> "${i}" ((ret+=$?)) done
>
> why loop ? `doins "${1}"/*.{png,svg}` works just as well
>
> you probably want to enable nullglobbing here, otherwise this will
> cause problems if you try to doicon on a dir that contains just
> svg.
>
> also, what about other file types ? people install xpm, svgz, gif,
> and other file types ...
>
>> exit ${ret}
>
> bash masks error codes to [0..255], so all the ret updates should
> probably be changed to just: ret=1
>
> after all, i doubt anyone cares how many errors there were, just
> that one occurred. and while you're here, might want to make it
> auto die on failure like we've done with all our other helpers.
> -mike

Thanks, I'v implemented most of that, but your proposal about
non-duplicated list in case) has multiple problems. The only cases
that actually work with that snippet are:
16x16|22x22|24x24|32x32|36x36|48x48|64x64|72x72|96x96|scalable.
All others will fail (like 128x128 or just 48).
So it would end up like this:

case $1 in
-s|--size)
if [[ ${2:0:2}x${2:0:2} == "$2" ]] ; then
size=${2:0:2}
elif [[ ${2:0:3}x${2:0:3} == "$2" ]] ; then
size=${2:0:3}
else
size=${2}
fi
case ${size} in
16|22|24|32|36|48|64|72|96|128|192|256)
size=${size}x${size};;
scalable)
;;
*)
eerror "${size} is an unsupported icon size!"
exit 1;;
esac
shift 2;;
-t|--theme)


This does not really look cleaner than just using two lists. I would
prefer the latter for readability.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJPvX3MAAoJEFpvPKfnPDWzxvMH/16kN1Zkby6LHg2Ev7H2qNPh
ajbqVonTuuLnIVxEwXYXYABEkF+qwD5xnJPMEclvkn8FXAVerFeyaxJgBelldXnr
DJMHiPhz0umJaMfvAFrEsbIo5IrxKMTpMMj3fuu5ruQMrSboV4alPSM7l2haXZ5W
3TbfbFmWoQzft1DolDlFb38M0TtRko7viZ1KQJUZjxCEClh8tEiOrQVxR8xcoi33
MiwEVZlib4KnWetq3qGZdU+xRFi/yzUmtFVv0pfbYIV51w4KHoi8cD6OkpiVzLdI
bhWCmyDeKq6wOcfXfcfGKzYc+2M/hP8xkhiG3/KjDXe6FUzdG63+U1Wmu521VDM=
=Rn8t
-----END PGP SIGNATURE-----


vapier at gentoo

May 23, 2012, 5:30 PM

Post #13 of 22 (374 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On Wednesday 23 May 2012 20:16:12 hasufell wrote:
> Thanks, I'v implemented most of that, but your proposal about
> non-duplicated list in case) has multiple problems. The only cases
> that actually work with that snippet are:
> 16x16|22x22|24x24|32x32|36x36|48x48|64x64|72x72|96x96|scalable.
> All others will fail (like 128x128 or just 48).

so do:
size=
if [[ $2 == "scalable" ]] ; then
size=$2
elif [[ ${2%%x*}x${2%%x*} == "$2" ]] ; then
size=${2%%x*}
case ${size} in
16|22|24|32|36|48|64|72|96|128|192|256) ;;
*) size= ;;
esac
fi
-mike
Attachments: signature.asc (0.82 KB)


hasufell at gentoo

May 23, 2012, 5:39 PM

Post #14 of 22 (373 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/24/2012 02:30 AM, Mike Frysinger wrote:
> On Wednesday 23 May 2012 20:16:12 hasufell wrote:
>> Thanks, I'v implemented most of that, but your proposal about
>> non-duplicated list in case) has multiple problems. The only
>> cases that actually work with that snippet are:
>> 16x16|22x22|24x24|32x32|36x36|48x48|64x64|72x72|96x96|scalable.
>> All others will fail (like 128x128 or just 48).
>
> so do: size= if [[ $2 == "scalable" ]] ; then size=$2 elif [[
> ${2%%x*}x${2%%x*} == "$2" ]] ; then size=${2%%x*} case ${size} in
> 16|22|24|32|36|48|64|72|96|128|192|256) ;; *) size= ;; esac fi
> -mike

that will install
doicon -s 256x256 "${FILESDIR}"/${PN}.svg
into
/usr/share/icons/hicolor/256/apps/${PN}.svg

and

doicon -s 256 "${FILESDIR}"/${PN}.svg
into
/usr/share/pixmaps/${PN}.svg


I don't see the point in bothering with that.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJPvYNeAAoJEFpvPKfnPDWzXeEIAKAK+c/+xkt4UIS2xLK9SGMQ
U1DwEqtiiytYvpB84QYrYjnoEMZrZN76uv6GsFtDYQC1nS5PDJt6F8yhGldF5CWr
UrD12iiIVIi3gLvkWVyfdhX3gA4wn/CL8Vq00R2oIMjy00uTBcYUmFV9X7xJzIxz
zyXhZBsXSpdnqZ1x9+nc9m9zy77Y7FIwA1dR9bWhBsiYMshUjTtGlIBE3uzm6v4Z
qKzhwKoG67jKFQuMyu495VSGGjXIJ0wuNofA2GRWLYZc5xP2nmeG5Q70vpM0CRiI
5Y2epr8thqbX53tsIxyJKqSwvqHGIKItChD0av9saIUa2D0KaUEiRrRN+m6cJuM=
=YxUg
-----END PGP SIGNATURE-----


hasufell at gentoo

May 23, 2012, 5:43 PM

Post #15 of 22 (374 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/24/2012 02:30 AM, Mike Frysinger wrote:
> On Wednesday 23 May 2012 20:16:12 hasufell wrote:
>> Thanks, I'v implemented most of that, but your proposal about
>> non-duplicated list in case) has multiple problems. The only
>> cases that actually work with that snippet are:
>> 16x16|22x22|24x24|32x32|36x36|48x48|64x64|72x72|96x96|scalable.
>> All others will fail (like 128x128 or just 48).
>
> so do: size= if [[ $2 == "scalable" ]] ; then size=$2 elif [[
> ${2%%x*}x${2%%x*} == "$2" ]] ; then size=${2%%x*} case ${size} in
> 16|22|24|32|36|48|64|72|96|128|192|256) ;; *) size= ;; esac fi
> -mike

alright

this would work


-s|--size)
if [[ ${2%%x*}x${2%%x*} == "$2" ]] ; then
size=${2%%x*}
else
size=${2}
fi
case ${size} in
16|22|24|32|36|48|64|72|96|128|192|256)
size=${size}x${size};;
scalable)
;;
*)
eerror "${size} is an unsupported icon size!"
exit 1;;
esac
shift 2;;

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJPvYRDAAoJEFpvPKfnPDWz8DgIAIA3T7GLWeN1m+8vfyeaVMXj
7UWyBXEcCU5geYqi5KsuY8/0Mq0e47ER+EH3VU+CrbV+UlDxxbyzsF4cqMRq/Oe+
Zz5CmSv4vjAfOyVe9psChue9PnKQwe/w6sS+L9zBftIzCb8G48Eefiir7lp9p7HO
9M5KkLeJwThDkUfhNUKtbNdIf1clnT1GVNrvcPuuasARN/of4ClQVVRISXKKHZen
9yA4D876CxtyN0jBHn78pRg0kZwSPL3PqucIQjli+usvGONX+LWuc7uWFJk8hnHI
E9cJrbUvkSJZx07ajEK1maohJBkfZdVWWxCGtZk0T00Kd2Nyy7o9SFX1hXX3F4I=
=/kaW
-----END PGP SIGNATURE-----


hasufell at gentoo

May 23, 2012, 6:04 PM

Post #16 of 22 (376 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On 05/21/2012 01:24 AM, hasufell wrote:
> I want support for installing icons into the appropriate directories
> which are under /usr/share/icons/... and not just pixmaps.
>
> proposal attached + diff
>
> This should not break existing ebuilds. Tested a bit and open for review
> now.

next version
Attachments: eutils.eclass.24may (2.51 KB)


mgorny at gentoo

May 23, 2012, 10:53 PM

Post #17 of 22 (374 views)
Permalink
Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On Thu, 24 May 2012 02:43:47 +0200
hasufell <hasufell [at] gentoo> wrote:

> -s|--size)
> if [[ ${2%%x*}x${2%%x*} == "$2" ]] ; then
> size=${2%%x*}
> else
> size=${2}
> fi
> case ${size} in
> 16|22|24|32|36|48|64|72|96|128|192|256)
> size=${size}x${size};;
> scalable)
> ;;

Not that anyone would care enough to try to understand what magic is
happening there... Please reimplement in Perl.

--
Best regards,
Michał Górny
Attachments: signature.asc (0.31 KB)


vapier at gentoo

Jun 1, 2012, 3:49 PM

Post #18 of 22 (349 views)
Permalink
Re: Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On Wednesday 23 May 2012 21:04:42 hasufell wrote:
> # @FUNCTION: _iconins
> # @DESCRIPTION:
> # function for use in doicon and newicon

mark it @INTERNAL

> if [[ -z $size ]] ; then

${size}

> if [[ $function == doicon ]] ; then

${function}

> if [[ $function == newicon ]] ; then

${function}

> doicon() {
> local function=$FUNCNAME
> _iconins "$@"

passing the funcname in this way is kind of ugly. you could do:
_iconins ${FUNCNAME} "$@"

and then at the top of _iconins:
local funcname=$1; shift

i guess if we all agree this complication is useful, then fix the nits and
let's merge it
-mike
Attachments: signature.asc (0.82 KB)


hasufell at gentoo

Jun 1, 2012, 7:50 PM

Post #19 of 22 (349 views)
Permalink
Re: Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On 06/02/2012 12:49 AM, Mike Frysinger wrote:
> On Wednesday 23 May 2012 21:04:42 hasufell wrote:
>> # @FUNCTION: _iconins
>> # @DESCRIPTION:
>> # function for use in doicon and newicon
>
> mark it @INTERNAL
>
>> if [[ -z $size ]] ; then
>
> ${size}
>
>> if [[ $function == doicon ]] ; then
>
> ${function}
>
>> if [[ $function == newicon ]] ; then
>
> ${function}
>
>> doicon() {
>> local function=$FUNCNAME
>> _iconins "$@"
>
> passing the funcname in this way is kind of ugly. you could do:
> _iconins ${FUNCNAME} "$@"
>
> and then at the top of _iconins:
> local funcname=$1; shift
>
> i guess if we all agree this complication is useful, then fix the nits and
> let's merge it
> -mike


Yo, I hope this is it.

Haven't seen many arguments against it as it is an optional feature anyway.

Anyone got reservations left?
Attachments: eutils.eclass.02jun (2.52 KB)


hasufell at gentoo

Jun 1, 2012, 7:58 PM

Post #20 of 22 (349 views)
Permalink
Re: Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

The small typo in the comment section (fuqbar.svg -> fuqbar.png) has
been fixed :P


vapier at gentoo

Jun 1, 2012, 9:48 PM

Post #21 of 22 (346 views)
Permalink
Re: Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

On Friday 01 June 2012 22:50:10 hasufell wrote:
> On 06/02/2012 12:49 AM, Mike Frysinger wrote:
> > On Wednesday 23 May 2012 21:04:42 hasufell wrote:
> >> # @FUNCTION: _iconins
> >> # @DESCRIPTION:
> >> # function for use in doicon and newicon
> >
> > mark it @INTERNAL

what i meant here was:
# @FUNCTION: _iconins
# @INTERNAL
# @DESCRIPTION:
# function for use in doicon and newicon

you can run /usr/portage/app-portage/eclass-manpages/files/eclass-to-manpage.sh
and the eclass to see if the style is valid
-mike
Attachments: signature.asc (0.82 KB)


hasufell at gentoo

Jun 2, 2012, 7:54 AM

Post #22 of 22 (352 views)
Permalink
Re: Re: enhancement for doicon/newicon in eutils.eclass [In reply to]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/02/2012 06:48 AM, Mike Frysinger wrote:
> On Friday 01 June 2012 22:50:10 hasufell wrote:
>> On 06/02/2012 12:49 AM, Mike Frysinger wrote:
>>> On Wednesday 23 May 2012 21:04:42 hasufell wrote:
>>>> # @FUNCTION: _iconins # @DESCRIPTION: # function for use in
>>>> doicon and newicon
>>>
>>> mark it @INTERNAL
>
> what i meant here was: # @FUNCTION: _iconins # @INTERNAL #
> @DESCRIPTION: # function for use in doicon and newicon
>
> you can run
> /usr/portage/app-portage/eclass-manpages/files/eclass-to-manpage.sh
> and the eclass to see if the style is valid -mike

K, fixed it. Manpage seems ok with this version.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJPyikKAAoJEFpvPKfnPDWz0S8IALS+uqaXMt8rOQUXPFjy09nZ
3gK+qJB2m453X21HprWcHEVL4/Exk77wWIWe0uZlFxuRN83+CF39PBIv6Bvr82qe
k1jb1+tr2GIK6undHVJXvWOgNzQ1LWIcKL1LOC6gwXlpBXstD5KgyeLpp6Igu7tw
GEalfgf5AmZ0v9QFKfR404ucvDs5uzXY1YRaFq6ygEvPRFHSzg7r2cnYdufwqz/R
s6R3UpFSEkXh/8J5cvMvk5N70SytB7bPVYUtbRi8N1bA+J8M6Iz4kre2ubn5w3i5
9YcuBxeW89JcBQOpex6UqAGL6BH/l2OVyDZ5+JpToTZXVrcPih5Pc5ilw4wOw7M=
=wky2
-----END PGP SIGNATURE-----
Attachments: eutils.eclass.02jun (2.54 KB)
  eutils.eclass.diff.02jun (3.49 KB)

Gentoo dev RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.