public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-dev] [PATCH] app-portage/eclass-manpages: Add support for @DEFAULT-VALUE
@ 2017-04-30 21:37 kentnl
  2017-05-01 10:05 ` Ulrich Mueller
  2017-05-01 10:14 ` Michał Górny
  0 siblings, 2 replies; 5+ messages in thread
From: kentnl @ 2017-04-30 21:37 UTC (permalink / raw
  To: gentoo-dev; +Cc: vapier, tools-portage, Kent Fredric

From: Kent Fredric <kentnl@gentoo.org>

@DEFAULT-VALUE allows eclasses to document the default values they
will inject when eclass-to-manpage can't extract it.

When eclass-to-manpage *can* extract it, it adds a warning when
the extracted value is different from that declared, (but the
declared value still takes precedence)

Note: there is a pre-exisitng poorly documented hack where

  # FOO=VALUE

In a comment serves as a fallback for literal value parsing, which
can supplement DEFAULT-VALUE in a less clear way.

But due to the nature of this syntax, its not trivial to identify
which eclasses are, and aren't using it as variables are routinely
commented without intending them to be used as documentation.

Some such commented assignments lurk in @CODE examples, which are
surely not intended to be extracted as their values

Subsequently, if present, @DEFAULT-VALUE will also trump any such
commented assignments
---
 .../eclass-manpages/files/eclass-to-manpage.awk     | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/app-portage/eclass-manpages/files/eclass-to-manpage.awk b/app-portage/eclass-manpages/files/eclass-to-manpage.awk
index 0d41f96327..d6ed59efd9 100644
--- a/app-portage/eclass-manpages/files/eclass-to-manpage.awk
+++ b/app-portage/eclass-manpages/files/eclass-to-manpage.awk
@@ -40,6 +40,7 @@
 # [@DEFAULT_UNSET]
 # [@INTERNAL]
 # [@REQUIRED]
+# @DEFAULT-VALUE: <initial value>
 # @DESCRIPTION:
 # <required; blurb about this variable>
 # foo="<default value>"
@@ -49,6 +50,7 @@
 # [@DEFAULT_UNSET]
 # [@INTERNAL]
 # [@REQUIRED]
+# @DEFAULT-VALUE: <initial value>
 # @DESCRIPTION:
 # <required; blurb about this variable>
 # foo="<default value>"
@@ -283,6 +285,7 @@ function _handle_variable() {
 	default_unset = 0
 	internal = 0
 	required = 0
+	default_value = ""
 
 	# make sure people haven't specified this before (copy & paste error)
 	if (all_vars[var_name])
@@ -299,6 +302,10 @@ function _handle_variable() {
 			internal = 1
 		else if ($2 == "@REQUIRED")
 			required = 1
+		else if ($2 == "@DEFAULT-VALUE:") {
+			sub(/^# @[A-Z_]*:[[:space:]]*/,"")
+			default_value = $0
+		}
 		else
 			opts = 0
 	}
@@ -315,15 +322,21 @@ function _handle_variable() {
 		op = "?="
 		regex = "^[[:space:]]*:[[:space:]]*[$]{" var_name ":?=(.*)}"
 		val = gensub(regex, "\\1", 1, $0)
-		if (val == $0) {
-			if (default_unset + required + internal == 0)
+	}
+	if (default_value != "") {
+		if ( val != $0 && default_value != val )
+			warn( var_name ": extracted different from DEFAULT-VALUE: " default_value " <=> " val )
+		op  = "="
+		val = default_value
+	}
+	if ( val == $0 ) {
+		if (default_unset + required + internal == 0)
 				warn(var_name ": unable to extract default variable content: " $0)
 			val = ""
-		} else if (val !~ /^["']/ && val ~ / /) {
+	} else if (val !~ /^["']/ && val ~ / /) {
 			if (default_unset == 1)
 				warn(var_name ": marked as unset, but has value: " val)
 			val = "\"" val "\""
-		}
 	}
 	if (length(val))
 		val = " " op " \\fI" val "\\fR"
-- 
2.12.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [gentoo-dev] [PATCH] app-portage/eclass-manpages: Add support for @DEFAULT-VALUE
  2017-04-30 21:37 [gentoo-dev] [PATCH] app-portage/eclass-manpages: Add support for @DEFAULT-VALUE kentnl
@ 2017-05-01 10:05 ` Ulrich Mueller
  2017-05-01 13:35   ` Kent Fredric
  2017-05-01 10:14 ` Michał Górny
  1 sibling, 1 reply; 5+ messages in thread
From: Ulrich Mueller @ 2017-05-01 10:05 UTC (permalink / raw
  To: gentoo-dev; +Cc: vapier, tools-portage, Kent Fredric

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

>>>>> On Mon, 1 May 2017, kentnl  wrote:

> @@ -40,6 +40,7 @@
>  # [@DEFAULT_UNSET]
>  # [@INTERNAL]
>  # [@REQUIRED]
> +# @DEFAULT-VALUE: <initial value>

Please make this DEFAULT_VALUE, in oder to be consistent with the
existing DEFAULT_UNSET. Otherwise, nobody will be able to remember
when to use a hyphen and when an underscore. 

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [gentoo-dev] [PATCH] app-portage/eclass-manpages: Add support for @DEFAULT-VALUE
  2017-04-30 21:37 [gentoo-dev] [PATCH] app-portage/eclass-manpages: Add support for @DEFAULT-VALUE kentnl
  2017-05-01 10:05 ` Ulrich Mueller
@ 2017-05-01 10:14 ` Michał Górny
  2017-05-01 13:38   ` Kent Fredric
  1 sibling, 1 reply; 5+ messages in thread
From: Michał Górny @ 2017-05-01 10:14 UTC (permalink / raw
  To: gentoo-dev, kentnl; +Cc: vapier, tools-portage, Kent Fredric

Dnia 30 kwietnia 2017 23:37:41 CEST, kentnl@gentoo.org napisał(a):
>From: Kent Fredric <kentnl@gentoo.org>
>
>@DEFAULT-VALUE allows eclasses to document the default values they
>will inject when eclass-to-manpage can't extract it.
>
>When eclass-to-manpage *can* extract it, it adds a warning when
>the extracted value is different from that declared, (but the
>declared value still takes precedence)
>
>Note: there is a pre-exisitng poorly documented hack where
>
>  # FOO=VALUE
>
>In a comment serves as a fallback for literal value parsing, which
>can supplement DEFAULT-VALUE in a less clear way.
>
>But due to the nature of this syntax, its not trivial to identify
>which eclasses are, and aren't using it as variables are routinely
>commented without intending them to be used as documentation.
>
>Some such commented assignments lurk in @CODE examples, which are
>surely not intended to be extracted as their values
>
>Subsequently, if present, @DEFAULT-VALUE will also trump any such
>commented assignments
>---
>.../eclass-manpages/files/eclass-to-manpage.awk     | 21
>+++++++++++++++++----
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
>diff --git a/app-portage/eclass-manpages/files/eclass-to-manpage.awk
>b/app-portage/eclass-manpages/files/eclass-to-manpage.awk
>index 0d41f96327..d6ed59efd9 100644
>--- a/app-portage/eclass-manpages/files/eclass-to-manpage.awk
>+++ b/app-portage/eclass-manpages/files/eclass-to-manpage.awk
>@@ -40,6 +40,7 @@
> # [@DEFAULT_UNSET]
> # [@INTERNAL]
> # [@REQUIRED]
>+# @DEFAULT-VALUE: <initial value>

I think you meant to make it [optional].

> # @DESCRIPTION:
> # <required; blurb about this variable>
> # foo="<default value>"
>@@ -49,6 +50,7 @@
> # [@DEFAULT_UNSET]
> # [@INTERNAL]
> # [@REQUIRED]
>+# @DEFAULT-VALUE: <initial value>
> # @DESCRIPTION:
> # <required; blurb about this variable>
> # foo="<default value>"
>@@ -283,6 +285,7 @@ function _handle_variable() {
> 	default_unset = 0
> 	internal = 0
> 	required = 0
>+	default_value = ""
> 
> 	# make sure people haven't specified this before (copy & paste error)
> 	if (all_vars[var_name])
>@@ -299,6 +302,10 @@ function _handle_variable() {
> 			internal = 1
> 		else if ($2 == "@REQUIRED")
> 			required = 1
>+		else if ($2 == "@DEFAULT-VALUE:") {
>+			sub(/^# @[A-Z_]*:[[:space:]]*/,"")

Any reason you can't just eat_line?

>+			default_value = $0
>+		}
> 		else
> 			opts = 0
> 	}
>@@ -315,15 +322,21 @@ function _handle_variable() {
> 		op = "?="
> 		regex = "^[[:space:]]*:[[:space:]]*[$]{" var_name ":?=(.*)}"
> 		val = gensub(regex, "\\1", 1, $0)
>-		if (val == $0) {
>-			if (default_unset + required + internal == 0)
>+	}
>+	if (default_value != "") {
>+		if ( val != $0 && default_value != val )
>+			warn( var_name ": extracted different from DEFAULT-VALUE: "
>default_value " <=> " val )
>+		op  = "="
>+		val = default_value
>+	}
>+	if ( val == $0 ) {
>+		if (default_unset + required + internal == 0)
> 				warn(var_name ": unable to extract default variable content: " $0)
> 			val = ""
>-		} else if (val !~ /^["']/ && val ~ / /) {
>+	} else if (val !~ /^["']/ && val ~ / /) {
> 			if (default_unset == 1)
> 				warn(var_name ": marked as unset, but has value: " val)
> 			val = "\"" val "\""
>-		}
> 	}
> 	if (length(val))
> 		val = " " op " \\fI" val "\\fR"


-- 
Best regards,
Michał Górny (by phone)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [gentoo-dev] [PATCH] app-portage/eclass-manpages: Add support for @DEFAULT-VALUE
  2017-05-01 10:05 ` Ulrich Mueller
@ 2017-05-01 13:35   ` Kent Fredric
  0 siblings, 0 replies; 5+ messages in thread
From: Kent Fredric @ 2017-05-01 13:35 UTC (permalink / raw
  To: gentoo-dev

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

On Mon, 01 May 2017 12:05:06 +0200
Ulrich Mueller <ulm@gentoo.org> wrote:

> Please make this DEFAULT_VALUE, in oder to be consistent with the
> existing DEFAULT_UNSET. Otherwise, nobody will be able to remember
> when to use a hyphen and when an underscore. 

Initial version did use _, but mgorny seems to be pushing for the
change to happen in the
other direction.

So this is just "-" until we decide for sure what's happening there and
I'll follow whatever standard we decide upon.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [gentoo-dev] [PATCH] app-portage/eclass-manpages: Add support for @DEFAULT-VALUE
  2017-05-01 10:14 ` Michał Górny
@ 2017-05-01 13:38   ` Kent Fredric
  0 siblings, 0 replies; 5+ messages in thread
From: Kent Fredric @ 2017-05-01 13:38 UTC (permalink / raw
  To: gentoo-dev

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

On Mon, 01 May 2017 12:14:01 +0200
Michał Górny <mgorny@gentoo.org> wrote:

> Any reason you can't just eat_line?

Would require me to reflow the entire function, eat_line calls getline for you.

Here, that would mean it would call "getline", and then the loop would continue,
and then it would call getline a second time, losing a line of input.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-05-01 13:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-30 21:37 [gentoo-dev] [PATCH] app-portage/eclass-manpages: Add support for @DEFAULT-VALUE kentnl
2017-05-01 10:05 ` Ulrich Mueller
2017-05-01 13:35   ` Kent Fredric
2017-05-01 10:14 ` Michał Górny
2017-05-01 13:38   ` Kent Fredric

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox