darcs

Patch 1017 Resolve issue2250: tabbing in usageHelper - pad by max length of command name

Title Resolve issue2250: tabbing in usageHelper - pad by max length of command name
Superseder Nosy List bsrkaditya
Related Issues Badly aligned Subcommands for `darcs show --help`
View: 2250
Status accepted Assigned To bsrkaditya
Milestone

Created on 2013-02-04.12:03:48 by bsrkaditya, last changed 2013-06-20.18:26:08 by darcswatch. Tracked on DarcsWatch.

Files
File name Status Uploaded Type Edit Remove
resolve-issue2250_-tabbing-in-usagehelper-_-pad-by-max-length-of-command-name.dpatch bsrkaditya, 2013-02-04.13:09:18 application/octet-stream
resolve-issue2250_-tabbing-in-usagehelper-_-pad-by-max-length-of-command-name.dpatch bsrkaditya, 2013-06-20.07:54:24 application/octet-stream
resolve-issue2250_-variable-tabbing-in-usagehelper.dpatch bsrkaditya, 2013-02-04.12:03:48 application/octet-stream
See mailing list archives for discussion on individual patches.
Messages
msg16563 (view) Author: owst Date: 2013-02-04.12:39:25
Hi bsrkaditya!

> 1 patch for repository http://darcs.net/screened:
> 
> Mon Feb  4 17:29:05 IST 2013  bsrkaditya@gmail.com
>   * Resolve issue2250: variable tabbing in usageHelper

It'd be good to say how you fix the issue, perhaps:

"Resolve issue2250: tabbing in usageHelper - pad by max length of
command name"

>  usageHelper :: [CommandControl] -> String
> -usageHelper [] = ""
> -usageHelper (HiddenCommand _ : cs) = usageHelper cs
> -usageHelper (CommandData c : cs) = "  " ++ padSpaces (commandName c) 15
> +usageHelper xs = usageHelper' (maximum.f $ xs) xs
> +   where f (CommandData c: cs) = ((+2).length.commandName $ c): f cs
> +         f (_: cs) = f cs
> +         f [] = [15]
> +

`f' here is a poor choice for a function name - it's not clear what it's
doing
(short named helpers are fine, if their purpose is obvious from context)
I'm also going to be picky and suggest spacing around the . operator -
as per
the style guidelines.

Also, I don't like the f [] = [15] idiom - Maybe Int as a result type, and
fromMaybe would be better - it doesn't currently  make its intention clear.

> +usageHelper' :: Int -> [CommandControl] -> String
> +usageHelper' _ [] = ""
> +usageHelper' x (HiddenCommand _ : cs) = usageHelper' x cs
> +usageHelper' x (CommandData c : cs) = "  " ++ padSpaces (commandName c) x
>                                          ++ chompNewline
(commandDescription c)
> hunk ./src/Darcs/UI/Commands.hs 234
> -                                        ++ "\n" ++ usageHelper cs
> -usageHelper (GroupName n:cs) = "\n" ++ n ++ "\n" ++ usageHelper cs
> +                                        ++ "\n" ++ usageHelper' x cs
> +usageHelper' x (GroupName n:cs) = "\n" ++ n ++ "\n" ++ usageHelper' x cs

Consistency in style please - here you've not used space around the
binary `:',
whereas in the other cases of this function you have.

> -  commandDescription = " Report patch-index status",
> +  commandDescription = "Report patch-index status",

This should really be a separate patch, but that's probably being a bit
picky
:-) it's "come out of nowhere" w.r.t. the bug/patch titles.

Thanks for the patch!

Owen.
msg16851 (view) Author: gh Date: 2013-06-14.22:31:38
Hi bsrkaditya,

can you send an updated patch that takes Owen's comments into account?
msg16878 (view) Author: bsrkaditya Date: 2013-06-20.07:54:24
Hi,
 I have removed the redundant hunk.

Aditya
Attachments
msg16879 (view) Author: gh Date: 2013-06-20.18:22:52
Thanks, accepting it on behalf of owen's review.
msg16882 (view) Author: darcswatch Date: 2013-06-20.18:26:06
This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-92c82c9ae99f0462b84687f156d7f6a185b936cf
msg16883 (view) Author: darcswatch Date: 2013-06-20.18:26:08
This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/screened.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_screened.html#bundle-92c82c9ae99f0462b84687f156d7f6a185b936cf
History
Date User Action Args
2013-02-04 12:03:48bsrkadityacreate
2013-02-04 12:05:58bsrkadityasetissues: + Badly aligned Subcommands for `darcs show --help`
2013-02-04 12:39:25owstsetstatus: needs-screening -> followup-requested
assignedto: bsrkaditya
messages: + msg16563
nosy: + bsrkaditya
2013-02-04 13:09:18bsrkadityasetfiles: + resolve-issue2250_-tabbing-in-usagehelper-_-pad-by-max-length-of-command-name.dpatch
2013-02-04 13:23:48bsrkadityasetstatus: followup-requested -> needs-screening
2013-02-04 13:26:58bsrkadityasettitle: Resolve issue2250: variable tabbing in usageHelper -> Resolve issue2250: tabbing in usageHelper - pad by max length of command name
2013-06-14 22:31:39ghsetmessages: + msg16851
2013-06-20 07:54:24bsrkadityasetfiles: + resolve-issue2250_-tabbing-in-usagehelper-_-pad-by-max-length-of-command-name.dpatch
messages: + msg16878
2013-06-20 07:55:36darcswatchsetdarcswatchurl: http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-92c82c9ae99f0462b84687f156d7f6a185b936cf
2013-06-20 18:22:52ghsetstatus: needs-screening -> accepted
messages: + msg16879
2013-06-20 18:26:06darcswatchsetmessages: + msg16882
2013-06-20 18:26:08darcswatchsetmessages: + msg16883