[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Future-proofing "svn st" output

From: Gareth McCaughan <Gareth.McCaughan_at_pobox.com>
Date: 2002-08-06 04:35:48 CEST

On Monday 05 August 2002 5:39 pm, Karl Fogel wrote:

[I'd written:]
> > 4. Offer some separate way for third-party programs to discover
> > which columns they need. For instance, an option that makes
> > "svn st" output a list of field names and columns.
...
> I like this solution best, personally.
...
> > I can't quite shake off the feeling that I may be worrying
> > about a non-problem; but I also can't see why it's a non-problem.
> > Am I missing some obvious reason why none of this matters?
>
> I think we can't be sure yet whether it's a problem, because we
> don't know what the future holds for "svn st". But if we ever get
> into a corner, now at least we have some solutions ready.
>
> > I remark that the widths of the columns for "svn st -v" are
> > different in all three of (1) the output of "svn help status",
> > (2) the examples in the Handbook, and (3) the actual output.
> >
> :-) Oops.
>
> Ummm, care to send in a patch? (Hey, can't hurt to try...)

Is that just for the minor inconsistencies in the last paragraph,
or for the whole thing? The answer's "yeah, sure" in either case.

I have a proto-patch that implements solution #4 as described
above and passes "make check" (apart from the independent breakage
introduced when "svn help --quiet" was added) and a quick eyeball
check, but I also have a bunch of questions.

  - In two parallel places in the existing clients/cmdline/status.c,
    there's (a) "if (foo == SVN_INVALID_REVNUM)" and (b)
    "if (SVN_IS_VALID_REVNUM (bar))". Provided the only
    negative revnum that can occur is -1, they're equivalent;
    is that so? If they *are* equivalent, which is preferred
    stylistically?

  - The special things that go in the revision number fields when
    an actual revision number is appropriate seem rather inconsistent
    to me. For instance, a file resulting from an uncommitted "svn cp"
    or "svn mv" gets its revision number shown as " -", with the
    dash at the end of the field; but one whose revnum is
    SVN_INVALID_REVNUM (I'm not sure what would cause that)
    is " ? " with the "?" in position 2; except that for the
    "last changed revision", as distinct from the "local revision",
    it's " ? " with the "?" in position 4.

    Does this make any sense? It doesn't to me, but I'm sure
    whoever set it up did so with some coherent reason in mind.

  - Is there a particular reason why the author-of-last-change field
    is right-aligned? The Handbook shows it left-aligned; the user
    and group fields in the output of "ls" are left-aligned; I think
    left-aligned looks better ... but I'm only an egg, and I don't
    want to go messing up something that's already been thought out
    carefully.

  - The "last changed revision" is clipped to 6 characters wide.
    It's the *first* 6 characters of the revision number that get
    shown. That seems obviously wrong, if only because showing the
    *last* 6 you can identify revision numbers uniquely up to 1099999
    by looking for leading zeros :-). The "local revision" isn't
    clipped at all, but is written into a fixed-size buffer. That's
    obviously a bug. What is the nicest behaviour here? What I've
    implemented so far is to show the last 6 digits, always.

  - In "detailed" format, there's usually an extra line saying what
    the head revision is, but that isn't printed when it's
    SVN_INVALID_REVNUM (I'm not sure when that would happen).
    Would it be better if that last line were *always* printed,
    saying something like "No valid revisions." or whatever if
    appropriate? If so, what should that message say?

Bad news: the patch approximately doubles the length of status.c;
it may be a little overdesigned.

Good news: it should make changing the fields very straightforward;
it fixes some buffer-overflow problems; there are somewhat fewer
magic numbers; the design does a better job of saying everything
"once and only once" than before.

The user-visible effect of the patch is to add a new command-line
option "-d" ("--describe-only", though I shan't be offended if
someone objects to that name) for "svn status". "svn status -d -u"
will produce the following output:

-------------------- sample begins --------------------
 0.. 0 textstatus Status of file/directory contents
 1.. 1 propstatus Status of file/directory properties
 2.. 2 locked 'L' if file/directory is locked
 3.. 3 copied '+' if to be added with history
 7.. 7 update '*' if file/directory needs updating
11..16 workrev working revision number
20.... pathname File or directory being examined
--------------------- sample ends ---------------------

and you should be able to extrapolate from there :-).

Argle, what a lot of fuss about such an unimportant matter. Sorry,
folks. I was going to leave off posting the current state of the
patch until I'd had answers to those questions, but I've changed
my mind. So here it is. At present it left-aligns author names,
which is a change from the old behaviour. Is it a problem that
"make check" doesn't catch this?

The patch is against revision 2890.

-------------------- patch begins --------------------
Index: ./subversion/clients/cmdline/cl.h
===================================================================
--- ./subversion/clients/cmdline/cl.h
+++ ./subversion/clients/cmdline/cl.h Tue Aug 6 01:43:29 2002
@@ -82,6 +82,7 @@
   svn_boolean_t verbose; /* be verbose */
   svn_boolean_t update; /* contact the server for the full
story */
   svn_boolean_t strict; /* do strictly what was requested */
+ svn_boolean_t describe_only; /* describe output format, don't
stat */
   svn_stringbuf_t *filedata; /* contents of file used as option
data */
   const char *filedata_encoding; /* the locale/encoding of the
filedata*/
   svn_boolean_t help; /* print usage message */
@@ -267,6 +268,13 @@
                                 svn_boolean_t show_last_committed,
                                 svn_boolean_t skip_unrecognized,
                                 apr_pool_t *pool);
+
+/* Print descriptions of all the fields that would be output
+ by calling svn_cl__print_status_list with parameters
+ DETAILED and SHOW_LAST_COMMITTED having specified values. */
+void svn_cl__describe_status_list (svn_boolean_t detailed,
+ svn_boolean_t
show_last_committed);
+
 
 /* Print a hash that maps property names (char *) to property values
    (svn_stringbuf_t *). The names are assumed to be in UTF-8 format;
Index: ./subversion/clients/cmdline/status.c
===================================================================
--- ./subversion/clients/cmdline/status.c
+++ ./subversion/clients/cmdline/status.c Tue Aug 6 03:02:46 2002
@@ -21,6 +21,10 @@
 
  
 /*** Includes. ***/
+#include <assert.h>
+#include <stdio.h>
+#include <string.h>
+
 #include <apr_hash.h>
 #include <apr_tables.h>
 #include "svn_sorts.h"
@@ -30,196 +34,435 @@
 #include "cl.h"
 
 
-/* Fill in the first four characters of STR_STATUS with status code
- characters, based on TEXT_STATUS, PROP_STATUS, LOCKED, and COPIED.
+
+/* A single item to be reported by some variant of "svn st".
+ The FILL_IN function will be passed a pointer to the start of the
field
+ and guarantees never to touch any characters outside the range
0..WIDTH-1.
+ On entrance to the function, the field is entirely filled with
spaces.
+ WIDTH is the width of the "data" portion of the column and doesn't
+ include white space between columns.
+ TAG is a shortish string, containing no whitespace, that another
+ program can use to recognize a field it's interested in. Tags
+ must never change, so choose them wisely.
+ DESC is a string containing a brief human-readable description
+ of the field.
+ Preferably, TAG should be at most 10 characters and DESC at most
60.
+ Then everything will fit in 2+2+2+1+10+1+60 = 77 characters. */
+struct status_item {
+ size_t width;
+ const char *tag;
+ const char *desc;
+ void (*fill_in)(char *, const svn_wc_status_t *);
+};
+
+/* We group status items (see above) together. Each group
+ consists of a list of items, each coupled with a number
+ of spaces to precede it by. The last item in the group
+ represents the first item in the next group; its SPACES_BEFORE
+ and ITEM should both be 0. For other items, ITEM points to
+ the status_item that tells how to format it, and SPACES_BEFORE
+ is, er, the number of spaces to precede it by. */
+struct grouped_status_item {
+ size_t spaces_before;
+ const struct status_item *item;
+};
+
+/* And, finally, we specify the format of a whole line
+ as a sequence of groups of status items. Each group
+ is defined by an array of grouped_status_item objects,
+ and is represented by a pointer to the first element
+ of the array. For each group, we also need a SPACES_BEFORE
+ count with the same meaning as in a grouped_status_item. */
+struct status_fields {
+ size_t spaces_before;
+ const struct grouped_status_item *group;
+};
+
+typedef struct status_item status_item_t;
+typedef struct grouped_status_item grouped_status_item_t;
+typedef struct status_fields status_fields_t;
+
+
+
+/* Some of the fill-in functions below nede to be able to write
+ a revision number into a fixed-width field of a string. This
+ is much more painful than it should be, so it's pulled out into
+ this function here. The number is right-aligned within the field;
+ if it won't fit, digits are thrown away at the left-hand end.
+ FIELD is assumed pre-filled with spaces. WIDTH is the number
+ of characters available. We never write to anywhere outside
+ the range 0..WIDTH-1. */
+static void
+fill_in__revnum (char *field, size_t width, svn_revnum_t rev)
+{
+ char temp[21]; /* enough space for an unsigned long, including
terminator */
 
- This function is also used by commit-cmd.c
-*/
-void
-svn_cl__generate_status_codes (char *str_status,
- enum svn_wc_status_kind text_status,
- enum svn_wc_status_kind prop_status,
- svn_boolean_t locked,
- svn_boolean_t copied)
-{
- char text_statuschar, prop_statuschar;
-
- switch (text_status)
- {
- case svn_wc_status_none:
- text_statuschar = ' ';
- break;
- case svn_wc_status_normal:
- text_statuschar = '_';
- break;
- case svn_wc_status_added:
- text_statuschar = 'A';
- break;
- case svn_wc_status_absent:
- text_statuschar = '!';
- break;
- case svn_wc_status_deleted:
- text_statuschar = 'D';
- break;
- case svn_wc_status_replaced:
- text_statuschar = 'R';
- break;
- case svn_wc_status_modified:
- text_statuschar = 'M';
- break;
- case svn_wc_status_merged:
- text_statuschar = 'G';
- break;
- case svn_wc_status_conflicted:
- text_statuschar = 'C';
- break;
- case svn_wc_status_unversioned:
- default:
- text_statuschar = '?';
- break;
- }
-
- switch (prop_status)
- {
- case svn_wc_status_none:
- prop_statuschar = ' ';
- break;
- case svn_wc_status_normal:
- prop_statuschar = '_';
- break;
- case svn_wc_status_added:
- prop_statuschar = 'A';
- break;
- case svn_wc_status_absent:
- prop_statuschar = '!';
- break;
- case svn_wc_status_deleted:
- prop_statuschar = 'D';
- break;
- case svn_wc_status_replaced:
- prop_statuschar = 'R';
- break;
- case svn_wc_status_modified:
- prop_statuschar = 'M';
- break;
- case svn_wc_status_merged:
- prop_statuschar = 'G';
- break;
- case svn_wc_status_conflicted:
- prop_statuschar = 'C';
- break;
- case svn_wc_status_unversioned:
- default:
- prop_statuschar = '?';
- break;
- }
-
- sprintf (str_status, "%c%c%c%c",
- text_statuschar,
- prop_statuschar,
- locked ? 'L' : ' ',
- copied ? '+' : ' ');
-}
-
-
-/* Print a single status structure in the short format */
-static void
-print_short_format (const char *path,
- svn_wc_status_t *status)
+ /* If WIDTH <= 20, we must copy the last WIDTH (non-\0) characters
+ of TEMP to FIELD. If WIDTH > 20, we must copy all of TEMP to
+ the last 20 characters of FIELD. So, COPY_LENGTH is the number
+ of characters to copy across, TEMP_OFFSET is the starting offset
+ within TEMP, and FIELD_OFFSET is the starting offset within
FIELD. */
+ size_t copy_length = width <= 20 ? width : 20;
+ size_t temp_offset = 20 - copy_length;
+ size_t field_offset = width - copy_length;
+
+ sprintf (temp, "%20lu", (unsigned long) rev);
+
+ memcpy (field+field_offset, temp+temp_offset, copy_length);
+}
+
+/* Perform the work of a fill-in function for the status of the
+ text or properties of a node. */
+static char
+fill_in__status (enum svn_wc_status_kind status)
 {
- char str_status[5];
+ switch (status)
+ {
+ case svn_wc_status_none: return ' ';
+ case svn_wc_status_normal: return '_';
+ case svn_wc_status_added: return 'A';
+ case svn_wc_status_absent: return '!';
+ case svn_wc_status_deleted: return 'D';
+ case svn_wc_status_replaced: return 'R';
+ case svn_wc_status_modified: return 'M';
+ case svn_wc_status_merged: return 'G';
+ case svn_wc_status_conflicted: return 'C';
+ case svn_wc_status_unversioned: return '?';
+ default: return '?';
+ }
+}
 
- if (! status)
- return;
 
- /* Create local-mod status code block. */
- svn_cl__generate_status_codes (str_status,
- status->text_status,
- status->prop_status,
- status->locked,
- status->copied);
-
- printf ("%s %s\n", str_status, path);
+
+/* Status item: text status. */
+
+/* Fill-in function (see definition of STATUS_ITEM above)
+ for the status of the content of a node. */
+static void
+fill_in_text_status (char *field, const svn_wc_status_t *status)
+{
+ field[0] = fill_in__status (status->text_status);
 }
 
+static status_item_t
+text_status_item = { 1, "textstatus", "Status of file/directory
contents",
+ fill_in_text_status };
+
+/* Status item: properties status. */
+
+/* Fill-in function (see definition of STATUS_ITEM above)
+ for the status of the properties of a node. */
+static void
+fill_in_prop_status (char *field, const svn_wc_status_t *status)
+{
+ field[0] = fill_in__status (status->prop_status);
+}
 
-/* Print a single status structure in the long format */
-static void
-print_long_format (const char *path,
- svn_boolean_t show_last_committed,
- svn_wc_status_t *status)
-{
- char str_status[5];
- char str_rev[7];
- char update_char;
- svn_revnum_t local_rev;
- char last_committed[6 + 3 + 8 + 3 + 1] = { 0 };
+static status_item_t
+prop_status_item = { 1, "propstatus", "Status of file/directory
properties",
+ fill_in_prop_status };
+
+/* Status item: locks. */
+
+/* Fill-in function (see definition of STATUS_ITEM above)
+ for the lockedness of a node. */
+static void
+fill_in_lockedness (char *field, const svn_wc_status_t *status)
+{
+ if (status->locked)
+ field[0] = 'L';
+}
 
- if (! status)
+static status_item_t
+lockedness_status_item = { 1, "locked", "'L' if file/directory is
locked",
+ fill_in_lockedness };
+
+/* Status item: copiedness. */
+
+/* Fill-in function (see definition of STATUS_ITEM above)
+ for the copiedness-with-history of a node. */
+static void
+fill_in_copiedness (char *field, const svn_wc_status_t *status)
+{
+ if (status->copied)
+ field[0] = '+';
+}
+
+static status_item_t
+copiedness_status_item = { 1, "copied", "'+' if to be added with
history",
+ fill_in_copiedness };
+
+/* Status item: out-of-dateness. */
+
+/* Fill-in function (see definition of STATUS_ITEM above)
+ for the out-of-dateness of a node. */
+static void
+fill_in_out_of_dateness (char *field, const svn_wc_status_t *status)
+{
+ if ((status->repos_text_status != svn_wc_status_none)
+ || (status->repos_prop_status != svn_wc_status_none))
+ field[0] = '*';
+}
+
+static status_item_t
+update_status_item = { 1, "update", "'*' if file/directory needs
updating",
+ fill_in_out_of_dateness };
+
+#define REV_WIDTH 6
+#define AUTHOR_WIDTH 8
+
+/* Status item: working revision. */
+
+/* Fill-in function (see definition of STATUS_ITEM above)
+ for the working revision of a node. */
+static void
+fill_in_working_rev (char *field, const svn_wc_status_t *status)
+{
+ if (! status->entry)
     return;
 
- /* Create local-mod status code block. */
- svn_cl__generate_status_codes (str_status,
- status->text_status,
- status->prop_status,
- status->locked,
- status->copied);
-
- /* Get local revision number */
- if (status->entry)
- local_rev = status->entry->revision;
+ if (status->entry->revision == SVN_INVALID_REVNUM)
+ field[REV_WIDTH-1] = '?';
+ else if (status->copied)
+ field[REV_WIDTH-1] = '-';
   else
- local_rev = SVN_INVALID_REVNUM;
+ fill_in__revnum (field, REV_WIDTH, status->entry->revision);
+}
 
- if (show_last_committed && status->entry)
- {
- char revbuf[20];
- const char *revstr = revbuf;
- const char *author;
-
- author = status->entry->cmt_author;
- if (SVN_IS_VALID_REVNUM (status->entry->cmt_rev))
- sprintf(revbuf, "%" SVN_REVNUM_T_FMT,
status->entry->cmt_rev);
- else
- revstr = " ? ";
-
- /* ### we shouldn't clip the revstr and author, but that
implies a
- ### variable length 'last_committed' which means an
allocation,
- ### which means a pool, ... */
- sprintf (last_committed, "%6.6s %8.8s ",
- revstr,
- author ? author : " ? ");
- }
+static status_item_t
+wrev_status_item = { REV_WIDTH, "workrev", "working revision number",
+ fill_in_working_rev };
+
+/* Status item: revision number of last change. */
+
+/* Fill-in function (see definition of STATUS_ITEM above)
+ for the revision number in which a node last changed. */
+static void
+fill_in_changed_rev (char *field, const svn_wc_status_t *status)
+{
+ if (! status->entry)
+ return;
+
+ if (status->entry->cmt_rev == SVN_INVALID_REVNUM)
+ field[REV_WIDTH-1] = '?';
   else
- strcpy (last_committed, " ");
+ fill_in__revnum (field, REV_WIDTH, status->entry->cmt_rev);
+}
 
- /* Set the update character. */
- update_char = ' ';
- if ((status->repos_text_status != svn_wc_status_none)
- || (status->repos_prop_status != svn_wc_status_none))
- update_char = '*';
+static status_item_t
+crev_status_item = { REV_WIDTH, "changerev", "revision number of last
change",
+ fill_in_changed_rev };
+
+/* Status item: author of last change. */
+
+/* Fill-in function (see definition of STATUS_ITEM above)
+ for the author of the last change to a node. */
+static void
+fill_in_author (char *field, const svn_wc_status_t *status)
+{
+ const char * author = 0;
 
- /* Determine the appropriate local revision string. */
   if (! status->entry)
- strcpy (str_rev, " ");
- else if (local_rev == SVN_INVALID_REVNUM)
- strcpy (str_rev, " ? ");
- else if (status->copied)
- strcpy (str_rev, " -");
+ return;
+
+ author = status->entry->cmt_author;
+ if (! author)
+ field[0] = '?';
   else
- sprintf (str_rev, "%6ld", local_rev);
+ {
+ size_t length = strlen (author);
+ size_t copy_length = length <= AUTHOR_WIDTH ? length :
AUTHOR_WIDTH;
 
- /* One Printf to rule them all, one Printf to bind them..." */
- printf ("%s %c %s %s%s\n",
- str_status,
- update_char,
- str_rev,
- show_last_committed ? last_committed : "",
- path);
+ memcpy (field, author, copy_length);
+ }
+}
+
+static status_item_t
+author_status_item = { AUTHOR_WIDTH, "author", "author of last
change",
+ fill_in_author };
+
+
+
+/* Fields for all variants of "svn status". */
+static grouped_status_item_t
+basic_fields[] = {
+ { 0, &text_status_item },
+ { 0, &prop_status_item },
+ { 0, &lockedness_status_item },
+ { 0, &copiedness_status_item },
+ { 0, 0 }
+};
+
+/* Fields describing the current rev and out-of-date-ness of a node.
*/
+static grouped_status_item_t
+update_fields[] = {
+ { 0, &update_status_item },
+ { 3, &wrev_status_item },
+ { 0, 0 }
+};
+
+/* Same as UPDATE_FIELDS but without the costly out-of-date flag. */
+static grouped_status_item_t
+cheap_update_fields[] = {
+ { 4, &wrev_status_item },
+ { 0, 0 }
+};
+
+/* Fields describing the last change to a node. */
+static grouped_status_item_t
+change_fields[] = {
+ { 0, &crev_status_item },
+ { 3, &author_status_item },
+ { 0, 0 }
+};
+
+
+
+/* Fields for plain "svn status". */
+static status_fields_t
+plain_fields[] = {
+ { 0, basic_fields },
+ { 3, 0 }
+};
+
+/* Fields for "svn status -u". */
+static status_fields_t
+minus_u_fields[] = {
+ { 0, basic_fields },
+ { 3, update_fields },
+ { 3, 0 }
+};
+
+/* Fields for "svn status -v". */
+static status_fields_t
+minus_v_fields[] = {
+ { 0, basic_fields },
+ { 3, cheap_update_fields },
+ { 3, change_fields },
+ { 3, 0 }
+};
+
+/* Fields for "svn status -u -v". */
+static status_fields_t
+minus_uv_fields[] = {
+ { 0, basic_fields },
+ { 3, update_fields },
+ { 3, change_fields },
+ { 3, 0 }
+};
+
+
+
+/* Run through all the items in a group, filling stuff in
+ starting at BUFFER. If BUFFER is a null pointer, don't
+ bother filling anything in. Return the total width we've
+ moved past in doing this, including gaps at start and end.
+ Assume BUFFER is large enough and is full of spaces already.
+ If BUFFER is non-null, STATUS must be too. If BUFFER is null,
+ STATUS is unused.*/
+static size_t
+traverse_group (char *buffer,
+ const grouped_status_item_t *group,
+ const svn_wc_status_t *status)
+{
+ size_t offset = 0;
+
+ while (group->item)
+ {
+ offset += group->spaces_before;
+ if (buffer)
+ group->item->fill_in (buffer+offset, status);
+ offset += group->item->width;
+ ++group;
+ }
+
+ /* We've reached the end, and GROUP->ITEM is null. But its
+ SPACES_BEFORE still needs applying. */
+ return offset + group->spaces_before;
+}
+
+/* Like TRAVERSE_GROUP above, but for a whole STATUS_FIELDS object.
*/
+static size_t
+traverse_fields (char *buffer,
+ const status_fields_t *fields,
+ const svn_wc_status_t *status)
+{
+ size_t offset = 0;
+
+ while (fields->group)
+ {
+ offset += fields->spaces_before;
+ offset += traverse_group (buffer ? buffer+offset : buffer,
+ fields->group, status);
+ ++fields;
+ }
+
+ /* We've reached the end, and FIELDS->GROUP is null. But its
+ SPACES_BEFORE still needs applying. */
+ return offset + fields->spaces_before;
+}
+
+/* Output a single status line for a given field set. */
+static void
+print_status_line (const char *path,
+ const status_fields_t *fields,
+ const svn_wc_status_t *status)
+{
+ char prefix[81];
+ size_t prefix_length = traverse_fields (0, fields, 0);
+ assert (prefix_length <= 80);
+
+ memset (prefix, ' ', prefix_length);
+ prefix[prefix_length] = '\0';
+
+ traverse_fields (prefix, fields, status);
+ printf ("%s%s\n", prefix, path);
+}
+
+
+
+/* Output field descriptions for all fields in a group,
+ assuming that the space-before the group starts at position
OFFSET.
+ Return the offset after processing the group. */
+static size_t
+describe_group (size_t offset, const grouped_status_item_t *group)
+{
+ while (group->item)
+ {
+ offset += group->spaces_before;
+ assert (offset+group->item->width < 100);
+ printf ("%2lu..%2lu %-10s %s\n",
+ (unsigned long) offset,
+ (unsigned long) offset+group->item->width-1,
+ group->item->tag, group->item->desc);
+ offset += group->item->width;
+ ++group;
+ }
+
+ /* We've reached the end, and GROUP->ITEM is null. But its
+ SPACES_BEFORE still needs applying. */
+ return offset + group->spaces_before;
 }
 
+/* Output field descriptions for all fields on a line. */
+static void
+describe_fields (const status_fields_t *fields)
+{
+ size_t offset = 0;
 
+ while (fields->group)
+ {
+ offset += fields->spaces_before;
+ offset = describe_group (offset, fields->group);
+ ++fields;
+ }
+
+ printf ("%2lu.... %-10s %s\n",
+ (unsigned long) offset+fields->spaces_before,
+ "pathname", "File or directory being examined");
+}
 
+
 /* Called by status-cmd.c */
 void
 svn_cl__print_status_list (apr_hash_t *statushash,
@@ -231,13 +474,20 @@
 {
   int i;
   apr_array_header_t *statusarray;
- svn_wc_status_t *status = NULL;
+ const svn_wc_status_t *status = NULL;
+ const status_fields_t *fields = NULL;
 
   /* Convert the unordered hash to an ordered, sorted array */
   statusarray = apr_hash_sorted_keys (statushash,
                                       
svn_sort_compare_items_as_paths,
                                       pool);
 
+ /* Identify the fieldset we need */
+ if (show_last_committed)
+ fields = detailed ? minus_uv_fields : minus_v_fields ;
+ else
+ fields = detailed ? minus_u_fields : plain_fields ;
+
   /* Loop over array, printing each name/status-structure */
   for (i = 0; i < statusarray->nelts; i++)
     {
@@ -255,16 +505,29 @@
       if (err)
         svn_handle_error (err, stderr, FALSE);
 
- if (detailed)
- print_long_format (path, show_last_committed, status);
- else
- print_short_format (path, status);
+ print_status_line (path, fields, status);
     }
 
   /* If printing in detailed format, we might have a head revision to
      print as well. */
   if (detailed && (youngest != SVN_INVALID_REVNUM))
     printf ("Head revision: %6ld\n", youngest);
+}
+
+/* Called by status-cmd.c */
+void
+svn_cl__describe_status_list (svn_boolean_t detailed,
+ svn_boolean_t show_last_committed)
+{
+ const status_fields_t *fields = NULL;
+
+ /* Identify the fieldset we need */
+ if (show_last_committed)
+ fields = detailed ? minus_uv_fields : minus_v_fields ;
+ else
+ fields = detailed ? minus_u_fields : plain_fields ;
+
+ describe_fields (fields);
 }
 
 
Index: ./subversion/clients/cmdline/main.c
===================================================================
--- ./subversion/clients/cmdline/main.c
+++ ./subversion/clients/cmdline/main.c Tue Aug 6 01:43:32 2002
@@ -75,6 +75,7 @@
     {"strict", svn_cl__strict_opt, 0, "use strict semantics"},
     {"no-ignore", svn_cl__no_ignore_opt, 0,
                       "disregard default and svn:ignore property
ignores"},
+ {"describe-only", 'd', 0, "describe columns; don't really do
status"},
     {0, 0, 0}
   };
 
@@ -359,6 +360,7 @@
     " With no args, print only locally modified files (no network
access).\n"
     " With -u, add out-of-date information from server.\n"
     " With -v, print excessive information on every file.\n"
+ " With -d, describe the output format given the other flags and
quit.\n"
     "\n"
     "Decoding the status output, each column is one character
wide:\n"
     " 1st column: Add or delete file or directory, or change file
contents\n"
@@ -378,7 +380,7 @@
     " _ 965 938 kfogel
./autogen.sh\n"
     " _ * 965 922 sussman
./build.conf\n"
     " M 965 687 joe
./buildcheck.sh\n",
- { 'u', 'v', 'N', 'q',
+ { 'u', 'v', 'N', 'q', 'd',
       svn_cl__auth_username_opt, svn_cl__auth_password_opt,
       svn_cl__no_ignore_opt } },
   
@@ -994,6 +996,9 @@
         break;
       case 'q':
         opt_state.quiet = TRUE;
+ break;
+ case 'd':
+ opt_state.describe_only = TRUE;
         break;
       case svn_cl__xml_file_opt:
         err = svn_utf_cstring_to_utf8 (&opt_state.xml_file, opt_arg,
Index: ./subversion/clients/cmdline/status-cmd.c
===================================================================
--- ./subversion/clients/cmdline/status-cmd.c
+++ ./subversion/clients/cmdline/status-cmd.c Tue Aug 6 01:14:18 2002
@@ -47,6 +47,13 @@
   SVN_ERR (svn_cl__args_to_target_array (&targets, os, opt_state,
                                          FALSE, pool));
 
+ if (opt_state->describe_only)
+ {
+ svn_cl__describe_status_list ((opt_state->verbose ||
opt_state->update),
+ opt_state->verbose);
+ return SVN_NO_ERROR;
+ }
+
   /* Build an authentication object to give to libsvn_client. */
   auth_baton = svn_cl__make_auth_baton (opt_state, pool);
 
--------------------- patch ends ---------------------

The thing I'm least pleased with here is the deeply nested
structure for holding fields. The aim is to reduce duplication
of data and thereby make it harder for anyone changing this
to introduce inconsistencies, but it does come at a cost.
Suggestions for improving this would be welcome.

The alternative, of course, is to throw in a few hardcoded
messages and require anyone adding or changing fields to
keep them up to date. That would involve much less change
to the existing code; it would also feel dirty. :-)

I shall not be at all offended if told "No, that's too much
complexity. Throw it away and find something simpler".

-- 
g
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Aug 6 04:36:30 2002

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.