[gentoo-dev] [PATCH] app-portage/eclass-manpages: Add support for @DEFAULT-ASSUMED

[gentoo-dev] [PATCH] app-portage/eclass-manpages: Add support for @DEFAULT-ASSUMED

[kentnl]

From: Kent Fredric <kentnl@gentoo.org>

@DEFAULT-ASSUMED allows eclasses to document any implied value
that internal code will assume when the ENV var is undefined.

@DEFAULT-ASSUMED should typically be used in conjunction with
@DEFAULT-UNSET, but it can be used in conjunction with either
@DEFAULT-VALUE or normal value extraction.

For instance:

  @VARIABLE: DIST_TEST
  @DEFAULT-ASSUMED: "do parallel"

This inserts an additional suffix to the generated man page heading
line so it renders as follows:

  DIST_TEST     (UNSET -> "do parallel")

But indicates that the value itself is not explicitly set by the eclass
and ebuilds should not assume it to have a value.

For instance, upon seeing such an indication, ebuild authors should
be able to tell that doing

  DIST_TEST+=" network"

Would end up producing

  DIST_TEST=" network"

Not

  DIST_TEST="do parallel network"

This is primarily for usecases where the variable is not assigned
anywhere in the top level file, but consuming functions imply a value:

  has "parallel" ${DIST_TEST:-do parallel}
---
 app-portage/eclass-manpages/files/eclass-to-manpage.awk | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/app-portage/eclass-manpages/files/eclass-to-manpage.awk b/app-portage/eclass-manpages/files/eclass-to-manpage.awk
index d6ed59efd9..772ee867d8 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-ASSUMED: <interpreted value when not set>
 # @DEFAULT-VALUE: <initial value>
 # @DESCRIPTION:
 # <required; blurb about this variable>
@@ -50,6 +51,7 @@
 # [@DEFAULT_UNSET]
 # [@INTERNAL]
 # [@REQUIRED]
+# @DEFAULT-ASSUMED: <interpreted value when not set>
 # @DEFAULT-VALUE: <initial value>
 # @DESCRIPTION:
 # <required; blurb about this variable>
@@ -285,6 +287,7 @@ function _handle_variable() {
 	default_unset = 0
 	internal = 0
 	required = 0
+	default_assumed = ""
 	default_value = ""
 
 	# make sure people haven't specified this before (copy & paste error)
@@ -302,8 +305,12 @@ function _handle_variable() {
 			internal = 1
 		else if ($2 == "@REQUIRED")
 			required = 1
+		else if ($2 == "@DEFAULT-ASSUMED:") {
+			sub(/^# @[A-Z-]*:[[:space:]]*/,"")
+			default_assumed = $0
+		}
 		else if ($2 == "@DEFAULT-VALUE:") {
-			sub(/^# @[A-Z_]*:[[:space:]]*/,"")
+			sub(/^# @[A-Z-]*:[[:space:]]*/,"")
 			default_value = $0
 		}
 		else
@@ -343,6 +350,10 @@ function _handle_variable() {
 	if (required == 1)
 		val = val " (REQUIRED)"
 
+	if ( default_assumed != "" ) {
+		val = val " (UNSET -> \\fI" default_assumed "\\fR)"
+	}
+
 	if (internal == 1)
 		return ""
 
-- 
2.12.2

Re: [gentoo-dev] [PATCH] app-portage/eclass-manpages: Add support for @DEFAULT-ASSUMED

[Michał Górny]

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

On pon, 2017-05-01 at 09:38 +1200, kentnl@gentoo.org wrote:
> From: Kent Fredric <kentnl@gentoo.org>
> 
> @DEFAULT-ASSUMED allows eclasses to document any implied value
> that internal code will assume when the ENV var is undefined.
> 
> @DEFAULT-ASSUMED should typically be used in conjunction with
> @DEFAULT-UNSET, but it can be used in conjunction with either
> @DEFAULT-VALUE or normal value extraction.
> 
> For instance:
> 
>   @VARIABLE: DIST_TEST
>   @DEFAULT-ASSUMED: "do parallel"
> 
> This inserts an additional suffix to the generated man page heading
> line so it renders as follows:
> 
>   DIST_TEST     (UNSET -> "do parallel")
> 
> But indicates that the value itself is not explicitly set by the eclass
> and ebuilds should not assume it to have a value.
> 
> For instance, upon seeing such an indication, ebuild authors should
> be able to tell that doing
> 
>   DIST_TEST+=" network"
> 
> Would end up producing
> 
>   DIST_TEST=" network"
> 
> Not
> 
>   DIST_TEST="do parallel network"
> 
> This is primarily for usecases where the variable is not assigned
> anywhere in the top level file, but consuming functions imply a value:
> 
>   has "parallel" ${DIST_TEST:-do parallel}

Well, I don't think there's really a good reason to expose this
in an explicit tag. It's going to be a little bit confusing at least
(your rendering isn't immediately obvious for users), and I don't really
see the problem being solved here.

As far as I can see, it applies to quite a specific corner case, when:

a. you want to assume a default value for the variable,

b. the assumed default is simple enough to be expressed directly, i.e.
unconditional,

c. but at the same time you stil want to keep it unset in global scope
for some reason.

Even if you have a very good reason for all the three conditions to be
met, I think that in the majority of cases you will need to explain what
particular values mean.

That being the case, I don't really see an advantage of explicitly
listing default value with potentially confusing syntax when the same
problem was already solved in a readable and non-confusing way by
explaining it in the body.

In other words, I don't think that:

  DIST_TEST      (UNSET -> "do parallel")

is more readable than:

  DIST_TEST      (UNSET)
  ...
  If unset, "do parallel" is assumed.

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: [gentoo-dev] [PATCH] app-portage/eclass-manpages: Add support for @DEFAULT-ASSUMED

[Kent Fredric]

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

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

> In other words, I don't think that:
> 
>   DIST_TEST      (UNSET -> "do parallel")
> 
> is more readable than:
> 
>   DIST_TEST      (UNSET)
>   ...
>   If unset, "do parallel" is assumed.
> 
> -- 

(This time on list)

If I were to take a second approach, producing a map of specific values
for a varible and their interpretations, so that the manpage emitter
could elegantly turn them into a bullet-pointed list of some
description, would that be a worthwhile alternative?


  DIST_TEST

    Values:
      - UNSET          same as "do parallel"
      - "do ..."       enable tests
      - "parallel ..." enable tests and parallelism
      - "network  ..." don't attempt to suppress network tests
      - "verbose ..."  increase test verbosity


   GENTOO_DEPEND_ON_PERL

     Values:
       - UNSET          same as "yes"
       - "yes"          depend on perl, including a slot operator for rebuild
       - "noslotop"     depend on perl, but without including the slot operator

The main objective here for me is to encourage a more clear convention
for documenting the purpose of the variable that is consistent across
eclasses, that doesn't require full reading of the DESCRIPTION to
cherry pick understandings of various isolated parts.

( Mostly, as they're typically not well structured for skim reading )

You may note the value map for DIST_TEST as stated is more-or-less
already in place for perl-module.eclass, albeit its just a hand
formatted table.

I just figure we could do more with this tree-wide if the data was
declared up-front, ( like line-wrapping the description side, using
styles for the "key" parts, etc, )

But I won't waste time throwing together such an idea if its not likely
to be deemed useful.

( I don't even want to think about what the syntax would be to document
it atm, ewww )

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

Re: [gentoo-dev] [PATCH] app-portage/eclass-manpages: Add support for @DEFAULT-ASSUMED

[Kent Fredric]

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

On Tue, 2 May 2017 09:25:52 +1200
Kent Fredric <kentnl@gentoo.org> wrote:

> You may note the value map for DIST_TEST as stated is more-or-less
> already in place for perl-module.eclass, albeit its just a hand
> formatted table.

oh ... and this strategy kinda fails if you render the manpage to PDF.

Because you get a proportional width font for that, and so everything
stops lining up :)

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