[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Patch: Required fields on inital PR creation

From: Mel Hatzis
Subject: Re: Patch: Required fields on inital PR creation
Date: Mon, 28 Oct 2002 11:17:10 -0800
User-agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.0.0) Gecko/20020530

On 10/27/02 11:36, Yngve Svendsen wrote:
At 16:42 15.10.2002 -0700, Mel Hatzis wrote:

Please review (and hopefully accept) the following patch that fixes a bug
in the 'on-change require' functionality and also expands the Gnats grammar
to allow for "required input fields" upon initial PR creation.

1/ The "require { fields }" functionality that's part of the on-change edit options for a field is only doing a check for field_value == NULL right now
   (rather than also checking for empty strings). Since field values are
generally initialized to an empty string (upon creation), the 'require' clause is basically useless. The patch adds a check for field_value == "".

2/ It is extremely useful to allow the Gnats administrator to configure
Gnats so that a predefined set of fields must be assigned a value upon initial PR creation. Together with the field's own 'on-change require' clause, this
   allows for fields that can never have a null (blank) value.
Allowing the Gnats administrator to enforce non-null fields this way provides
   a powerful mechanism toward improved data integrity.

This doesn't work quite as it should with multitext fields. If a field is in the initial-entry fields list, both send-pr and Gnatsweb will include it when they submit PRs, even if it is empty. In the submitted PR text, these fields will contain newline character but nothing else. This slips past the require functionality that this patch implements.

Good catch...your patch looks entirely reasonable.

It'd be nice if the Gnats code didn't add newlines for otherwise
blank values - while this may make the display_pr functionality
a little easier, I think it's not very intuitive. Perhaps something
we can improve on for 4.1.


I suggest we also check for space characters, as defined by isspace(), by making the following trivial change:

Index: edit.c
RCS file: /cvsroot/gnats/gnats/gnats/edit.c,v
retrieving revision 1.58
diff -u -p -r1.58 edit.c
--- edit.c      24 Oct 2002 12:45:09 -0000      1.58
+++ edit.c      27 Oct 2002 19:29:43 -0000
@@ -1105,7 +1105,7 @@ applyChangeAction (ChangeActions action,
   while (fields != NULL)
const char *fldval = get_field_value (pr, oldPR, fields->ent, NULL, NULL);
-      if (fldval == NULL || *fldval == '\0')
+      if (value_is_empty (fldval))
          setError (err, CODE_INVALID_PR_CONTENTS,
                    "Required field %s missing from PR %s.",
Index: file-pr.c
RCS file: /cvsroot/gnats/gnats/gnats/file-pr.c,v
retrieving revision 1.48
diff -u -p -r1.48 file-pr.c
--- file-pr.c   24 Oct 2002 12:45:09 -0000      1.48
+++ file-pr.c   27 Oct 2002 19:29:43 -0000
@@ -446,7 +446,7 @@ missing_required_fields (const DatabaseI
   while (fields != NULL)
const char *fldval = get_field_value (pr, NULL, fields->ent, NULL, NULL);
-      if (fldval == NULL || *fldval == '\0')
+      if (value_is_empty (fldval))
           setError (err, CODE_INVALID_PR_CONTENTS,
"Required field %s missing from new PR - cannot submit.",

- Yngve

reply via email to

[Prev in Thread] Current Thread [Next in Thread]