[darcs-users] Request for comments: command description and documentation patch

David Roundy droundy at abridgegame.org
Mon Mar 7 13:06:43 UTC 2005


Peter sent a patch in with modifications of the descriptions of darcs'
commands, and I thought I'd prefer to have some third-party review here.
Most of the changes look like improvements to me, but a few I'm not so sure
about.  But I'd appreciate comments on any descriptions that anyone thinks
could use improvement.

On Sun, Mar 06, 2005 at 06:50:30PM +0100, Peter Hercek wrote:
> Hi,
> 
> the patch as specified here is attached:
> http://article.gmane.org/gmane.comp.version-control.darcs.user/5704
> 
> I did not do the grouping of commands. The only idea (I have) how
> to do it is to add an enum to each command structure which would
> specify its group. I'm not sure how it would look in Haskell yet but in
> OCAML the type would be like:
> type cmd_group =
>  Working_copy_manipulation of string
> | Working_copy_Local_repository_transfer of string
> | ....  ;;
> 
> If you like the proposal please reply, I may look at it sometimes.
> I work primary on windows and darcs seems a bit "rough" there
> (lately, I had some issues there with editor for long comments)
> so I'll try to find out whether it does not work because of my
> weak knowledge or whether these are errors.

I think it might be simpler to simply include in the DarcsCommand data
structure a string indicating the section heading for the command.  The
annoying bit would that we'd have that then if you rename a section you'd
have to modify all the commands in that section.  Either way, I agree that
an infrastructure change to support command grouping would be wise.

As a starting point for grouping the commands, however, I'd prefer the
grouping used in darcs.lhs, which is also the grouping used in the manual.
(Although perhaps with less silliness).

> [command description and help patch
> peter at syncad.com**20050306171212] {

Quoting the patch as a whole, since others may have comments on bits that I
don't see a problem with...

> hunk ./Annotate.lhs 62
> -annotate_description = "Display which patch last modified something, or display patch contents"
> +annotate_description = "Display which patch last modified something."
> hunk ./Annotate.lhs 69
> - "Annotate displays which patches created or last modified a directory file or line.\n"++
> - "It can also display the contents of a particular patch in darcs format.\n"
> + "Annotate displays which patches created or last modified a directory\n"++
> + "file or line. It can also display the contents of a particular patch\n"++
> + "in darcs format.\n"
> hunk ./Apply.lhs 75
> - "Apply patches to a repo."
> + "Apply patches (from the email bundle) to a repository."

>From *an* email bundle?

> hunk ./Get.lhs 55
> - "Get a repository."
> + "Create new local repository with content of remote one."

"Create a local copy of a repote repository"?

> hunk ./Mv.lhs 41
> - "Move one or more files or directories to a different location."
> + "Move/rename one or more files or directories."
> hunk ./Pull.lhs 69
> - "Pull patches from another repo."
> + "Copy patches from remote to local repository."

Here and later, I'm not sure I'm comfortable with the use of the word
"copy".  Copying a patch doesn't imply to me applying the patch to the
working directory.  Perhaps something like retrieve?  :( I haven't come up
with a better wording.

I'm also not sure about the general principle of using the name of a
command in its short description.  I see that I've often done so in the
existing short descriptions, and it does sort of seem like it might help in
understanding the sense in which the command name is meant, but on the
other hand, it might explain nothing to users...

> hunk ./Push.lhs 48
> - "Push patches into another repo."
> + "Copy patches from local to remote repository."
> hunk ./Push.lhs 56
> - "Push is the opposite of pull.  Push allows you to move changes from the\n"++
> + "Push is the opposite of pull.  Push allows you to copy changes from the\n"++
> hunk ./Record.lhs 64
> - "Record changes as a named patch."
> + "Save working copy changes to repository as a patch."
> hunk ./Record.lhs 69
> -Record is used to name a set of changes and record the patch to the
> -repository.  If you provide one or more files or directories as additional
> -arguments to record, you will only be prompted to changes in those files or
> +If you provide one or more files or directories as additional arguments
> +to record, you will only be prompted to changes in those files or
> hunk ./Record.lhs 75
> - "Record is used to name a set of changes.\n"
> + "Record is used to name a set of changes and record the patch to the\n"++
> + "repository.'n"
> hunk ./Remove.lhs 42
> - "Remove one or more files or directories from the repository."
> + "Remove one or more files or directories."
> hunk ./Resolve.lhs 40
> - "Resolve conflicts."
> + "Record the conflicting patch and its resolution."

This isn't what resolve does--it doesn't record a patch.  It really just
marks the working directory to indicate any conflicts that may not be
apparent.

> hunk ./Revert.lhs 47
> - "Revert to recorded version."
> + "Revert to recorded version (safe the first time only)."
> hunk ./Revert.lhs 61
> - "wish to undo.\n"
> + "wish to undo. The last revert can be undone safely using unrevert\n"++
> + "command if the working copy was not modified in the meantime.\n"

... using *the* unrevert command...

> hunk ./Rollback.lhs 43
> - "Roll back a named patch."
> + "Apply an inverse patch in repository to local copy."

I'm unclear here what local copy means... if it means the working
directory, then this description is incorrect.  Rollback doesn't affect the
working directory, but just "records" an inverse patch.

> hunk ./SetPref.lhs 34
> - "Set the value for a preferences thingy."
> + "Set the value for a preference (test, predist, ...)."
> hunk ./SetPref.lhs 75
> - "Setpref allows you to set a preferences value in a way that will\n"++
> + "Setpref allows you to set a preference value in a way that will\n"++
> hunk ./Tag.lhs 34
> - "Tag the contents of a repo with a given version name."
> + "Tag the contents of repository with a version name."
> hunk ./Tag.lhs 50
> - "Tag is used to name a version of the tree.\n"
> + "Tag is used to name a version of local repository (i.e. the whole tree).\n"
> hunk ./Unrecord.lhs 48
> - "Unrecord a named patch."
> + "Remove a recorded patch (without working copy change)."

I think working directory would be clearer than working copy (since copy
could also be used as a verb).  Perhaps "(without modifying working
directory)"

> hunk ./Unrecord.lhs 147
> - "Unpull a named patch."
> + "Opposite of pull; unsafe if the patch is not in remote repo."
> hunk ./Unrevert.lhs 53
> - "Undo the last revert operation."
> + "Undo the last revert (may fail if changes after the revert)."
> hunk ./Unrevert.lhs 62
> - "Unrevert is used to undo the results of a revert command.\n"
> + "Unrevert is used to undo the results of a revert command. It is only\n"++
> + "guaranteed to work properly if you haven't made any changes since the\n"++
> + "revert was performed.\n"
> hunk ./Unrevert.lhs 66
> -While this is only guaranteed to work properly if you haven't made any
> -changes since the revert was performed, it makes a best effort to merge the
> -unreversion with any changes you have since made.  In fact, unrevert should
> -even work if you've recorded changes since reverting.
> +The command makes a best effort to merge the unreversion with any changes
> +you have since made.  In fact, unrevert should even work if you've recorded
> +changes since reverting.
> hunk ./WhatsNew.lhs 58
> -whatsnew_description = "Display unrecorded changes in the working directory."
> +whatsnew_description = "Display unrecorded changes in working directory."

Thanks, Peter, for this contribution.  Documentation patches are always
appreciated!
-- 
David Roundy
http://www.darcs.net




More information about the darcs-users mailing list