[gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.

[gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.

[Patrice Clement]

... and replace it with a test against the REPLACING_VERSIONS variable.

See https://projects.gentoo.org/pms/8/pms.html#x1-10900011.1.

Patch courtesy of Arfrever.

Closes: https://github.com/gentoo/gentoo/pull/21733
Signed-off-by: Patrice Clement <monsieurp@gentoo.org>
---
 eclass/vim-plugin.eclass | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
index 6b72d66111d..460edcce3eb 100644
--- a/eclass/vim-plugin.eclass
+++ b/eclass/vim-plugin.eclass
@@ -126,31 +126,31 @@ update_vim_afterscripts() {
 display_vim_plugin_help() {
 	local h
 
-	if ! has_version ${CATEGORY}/${PN} ; then
-		if [[ -n "${VIM_PLUGIN_HELPFILES}" ]] ; then
+	if [[ -z ${REPLACING_VERSIONS} ]]; then
+		if [[ -n "${VIM_PLUGIN_HELPFILES}" ]]; then
 			elog " "
 			elog "This plugin provides documentation via vim's help system. To"
 			elog "view it, use:"
-			for h in ${VIM_PLUGIN_HELPFILES} ; do
+			for h in ${VIM_PLUGIN_HELPFILES}; do
 				elog "    :help ${h}"
 			done
 			elog " "
 
-		elif [[ -n "${VIM_PLUGIN_HELPTEXT}" ]] ; then
+		elif [[ -n "${VIM_PLUGIN_HELPTEXT}" ]]; then
 			elog " "
 			while read h ; do
 				elog "$h"
 			done <<<"${VIM_PLUGIN_HELPTEXT}"
 			elog " "
 
-		elif [[ -n "${VIM_PLUGIN_HELPURI}" ]] ; then
+		elif [[ -n "${VIM_PLUGIN_HELPURI}" ]]; then
 			elog " "
 			elog "Documentation for this plugin is available online at:"
 			elog "    ${VIM_PLUGIN_HELPURI}"
 			elog " "
 		fi
 
-		if has "filetype" "${VIM_PLUGIN_MESSAGES}" ; then
+		if has "filetype" "${VIM_PLUGIN_MESSAGES}"; then
 			elog "This plugin makes use of filetype settings. To enable these,"
 			elog "add lines like:"
 			elog "    filetype plugin on"
-- 
2.31.1

Re: [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 189 bytes --]

>>>>> On Wed, 21 Jul 2021, Patrice Clement wrote:

> +		if [[ -n "${VIM_PLUGIN_HELPFILES}" ]]; then

Quotation marks are not necessary here (and three more times below,
in the elif lines).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]

Re: [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]



> On 21 Jul 2021, at 17:47, Patrice Clement <monsieurp@gentoo.org> wrote:
> 
> ... and replace it with a test against the REPLACING_VERSIONS variable.
> 
> See https://projects.gentoo.org/pms/8/pms.html#x1-10900011.1.
> 
> Patch courtesy of Arfrever.
> 

Ideally, we'd use Arfrever's sign off, but this is kind of a grey
area, so it's not a blocker at all right now.

> Closes: https://github.com/gentoo/gentoo/pull/21733
> Signed-off-by: Patrice Clement <monsieurp@gentoo.org>
> ---
> eclass/vim-plugin.eclass | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
> index 6b72d66111d..460edcce3eb 100644
> --- a/eclass/vim-plugin.eclass
> +++ b/eclass/vim-plugin.eclass
> [snip]

The patch itself seems fine, but I have some suggestions while we're
working on the eclass:

- Add EAPI 8 support

- Please drop obsolete EAPI compatibility hacks like:
>has "${EAPI:-0}" 0 1 2 && ! use prefix && ED="${D}"

This is unnecessary because we're only declaring support for EAPI 6 and 7 right now.

- Could you please convert POSIX tests to Bash tests? [0]

- In update_vim_afterscripts, the einfo seems (at first glance) to be misleading. Should
it be printing ${afterdir} instead?

- Ideally, we'd add the standard inherit guard if possible (see e.g. gnuconfig.eclass for a simple example).

[0] https://devmanual.gentoo.org/tools-reference/bash/#single-versus-double-brackets-in-bash

Let me know if you need any assistance, etc.

thanks,
sam





[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.

[Patrice Clement]

Wednesday 21 Jul 2021 19:16:36, Ulrich Mueller wrote :
> >>>>> On Wed, 21 Jul 2021, Patrice Clement wrote:
> 
> > +		if [[ -n "${VIM_PLUGIN_HELPFILES}" ]]; then
> 
> Quotation marks are not necessary here (and three more times below,
> in the elif lines).

Thanks, this has been mended.

Re: [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.

[Patrice Clement]

Thursday 22 Jul 2021 04:44:39, Sam James wrote :
> 
> 
> > On 21 Jul 2021, at 17:47, Patrice Clement <monsieurp@gentoo.org> wrote:
> > 
> > ... and replace it with a test against the REPLACING_VERSIONS variable.
> > 
> > See https://projects.gentoo.org/pms/8/pms.html#x1-10900011.1.
> > 
> > Patch courtesy of Arfrever.
> > 
> 
> Ideally, we'd use Arfrever's sign off, but this is kind of a grey
> area, so it's not a blocker at all right now.
> 
> > Closes: https://github.com/gentoo/gentoo/pull/21733
> > Signed-off-by: Patrice Clement <monsieurp@gentoo.org>
> > ---
> > eclass/vim-plugin.eclass | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
> > index 6b72d66111d..460edcce3eb 100644
> > --- a/eclass/vim-plugin.eclass
> > +++ b/eclass/vim-plugin.eclass
> > [snip]
> 
> The patch itself seems fine, but I have some suggestions while we're
> working on the eclass:
> 
> - Add EAPI 8 support
> 
> - Please drop obsolete EAPI compatibility hacks like:
> >has "${EAPI:-0}" 0 1 2 && ! use prefix && ED="${D}"
> 
> This is unnecessary because we're only declaring support for EAPI 6 and 7 right now.
> 
> - Could you please convert POSIX tests to Bash tests? [0]
> 
> - In update_vim_afterscripts, the einfo seems (at first glance) to be misleading. Should
> it be printing ${afterdir} instead?
> 
> - Ideally, we'd add the standard inherit guard if possible (see e.g. gnuconfig.eclass for a simple example).
> 
> [0] https://devmanual.gentoo.org/tools-reference/bash/#single-versus-double-brackets-in-bash
> 
> Let me know if you need any assistance, etc.
> 
> thanks,
> sam
> 
> 
> 
> 

Thanks for the suggestions. If you don't mind, I'd like to stick to the
original changes for now and address your suggestions later in a follow up
patch.

Best,

[gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.

[Patrice Clement]

... and replace it with a test against the REPLACING_VERSIONS variable.

See https://projects.gentoo.org/pms/8/pms.html#x1-10900011.1.

This is an updated version of the same patch discussed previously on this ML.

Suggested by Arfrever.
---
 eclass/vim-plugin.eclass | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
index 6b72d66111d..460edcce3eb 100644
--- a/eclass/vim-plugin.eclass
+++ b/eclass/vim-plugin.eclass
@@ -126,31 +126,31 @@ update_vim_afterscripts() {
 display_vim_plugin_help() {
 	local h
 
-	if ! has_version ${CATEGORY}/${PN} ; then
-		if [[ -n "${VIM_PLUGIN_HELPFILES}" ]] ; then
+	if [[ -z ${REPLACING_VERSIONS} ]]; then
+		if [[ -n ${VIM_PLUGIN_HELPFILES} ]]; then
 			elog " "
 			elog "This plugin provides documentation via vim's help system. To"
 			elog "view it, use:"
-			for h in ${VIM_PLUGIN_HELPFILES} ; do
+			for h in ${VIM_PLUGIN_HELPFILES}; do
 				elog "    :help ${h}"
 			done
 			elog " "
 
-		elif [[ -n "${VIM_PLUGIN_HELPTEXT}" ]] ; then
+		elif [[ -n ${VIM_PLUGIN_HELPTEXT} ]]; then
 			elog " "
 			while read h ; do
 				elog "$h"
 			done <<<"${VIM_PLUGIN_HELPTEXT}"
 			elog " "
 
-		elif [[ -n "${VIM_PLUGIN_HELPURI}" ]] ; then
+		elif [[ -n ${VIM_PLUGIN_HELPURI} ]]; then
 			elog " "
 			elog "Documentation for this plugin is available online at:"
 			elog "    ${VIM_PLUGIN_HELPURI}"
 			elog " "
 		fi
 
-		if has "filetype" "${VIM_PLUGIN_MESSAGES}" ; then
+		if has filetype ${VIM_PLUGIN_MESSAGES}; then
 			elog "This plugin makes use of filetype settings. To enable these,"
 			elog "add lines like:"
 			elog "    filetype plugin on"
-- 
2.31.1

Re: [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]



> On 22 Jul 2021, at 09:32, Patrice Clement <monsieurp@gentoo.org> wrote:
> 
> Thursday 22 Jul 2021 04:44:39, Sam James wrote :
>> 
>>> [snip]
>> 
>> The patch itself seems fine, but I have some suggestions while we're
>> working on the eclass:
>> 
>> [snip]

>> Let me know if you need any assistance, etc.
>> 
>> thanks,
>> sam
>> 

> Thanks for the suggestions. If you don't mind, I'd like to stick to the
> original changes for now and address your suggestions later in a follow up
> patch.

It's preferred [0] where, if we know more changes are coming, to batch them up,
unless this is really critical. This is because of needless cache regeneration.

It's generally not a reason to block fixing something if no other issues exist, but it
is a reason to stop and pause/reflect if there are any other low-hanging fruit we could fix while there.

A lot of this is straightforward tidying. I don't mind if you're sure you will be able to return
to it shortly, but then I figure, why not just do it now?

[0] https://devmanual.gentoo.org/eclass-writing/index.html#adding-and-updating-eclasses (see the "Note")

thanks,
sam

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.

[Patrice Clement]

Thursday 22 Jul 2021 10:54:02, Patrice Clement wrote :
> ... and replace it with a test against the REPLACING_VERSIONS variable.
> 
> See https://projects.gentoo.org/pms/8/pms.html#x1-10900011.1.
> 
> This is an updated version of the same patch discussed previously on this ML.
> 
> Suggested by Arfrever.
> ---
>  eclass/vim-plugin.eclass | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
> index 6b72d66111d..460edcce3eb 100644
> --- a/eclass/vim-plugin.eclass
> +++ b/eclass/vim-plugin.eclass
> @@ -126,31 +126,31 @@ update_vim_afterscripts() {
>  display_vim_plugin_help() {
>  	local h
>  
> -	if ! has_version ${CATEGORY}/${PN} ; then
> -		if [[ -n "${VIM_PLUGIN_HELPFILES}" ]] ; then
> +	if [[ -z ${REPLACING_VERSIONS} ]]; then
> +		if [[ -n ${VIM_PLUGIN_HELPFILES} ]]; then
>  			elog " "
>  			elog "This plugin provides documentation via vim's help system. To"
>  			elog "view it, use:"
> -			for h in ${VIM_PLUGIN_HELPFILES} ; do
> +			for h in ${VIM_PLUGIN_HELPFILES}; do
>  				elog "    :help ${h}"
>  			done
>  			elog " "
>  
> -		elif [[ -n "${VIM_PLUGIN_HELPTEXT}" ]] ; then
> +		elif [[ -n ${VIM_PLUGIN_HELPTEXT} ]]; then
>  			elog " "
>  			while read h ; do
>  				elog "$h"
>  			done <<<"${VIM_PLUGIN_HELPTEXT}"
>  			elog " "
>  
> -		elif [[ -n "${VIM_PLUGIN_HELPURI}" ]] ; then
> +		elif [[ -n ${VIM_PLUGIN_HELPURI} ]]; then
>  			elog " "
>  			elog "Documentation for this plugin is available online at:"
>  			elog "    ${VIM_PLUGIN_HELPURI}"
>  			elog " "
>  		fi
>  
> -		if has "filetype" "${VIM_PLUGIN_MESSAGES}" ; then
> +		if has filetype ${VIM_PLUGIN_MESSAGES}; then
>  			elog "This plugin makes use of filetype settings. To enable these,"
>  			elog "add lines like:"
>  			elog "    filetype plugin on"
> -- 
> 2.31.1
> 
> 

Merged. Thanks ulm for the review!