Thanks for the feedback!

Updated with the suggested changes.


On 04/23/2017 06:28 PM, Noam Postavsky wrote:
>> Subject: [PATCH] update vc-git
>>
>> ---
> Could you add a meaningful commit message please?  See CONTRIBUTE (under
> "Commit messages") for details of Emacs' standard format.
>
> I think the patch is basically okay now, just a few minor nitpicks below.
>
>> +(defun vc-git--git-status-to-vc-state (code-list)
>> +  "Convert a list CODE-LIST of two-letter git status strings to a vc status.
> This line is too long, I think it should be fine to shorten to just
> "Convert CODE-LIST to a vc status".  You explain the format of
> CODE-LIST in the next paragraph anyway.
>
>> +Each element of CODE-LIST comes from the first two characters of
>> +a line returned by 'git status' and should be passed in the order given by 'git status'.
> This paragraph looks unfilled, hit M-q on it.
>
>> +       ;; I have only seen this with a file that is only present in the
>> +       ;; index.  Let us call this `removed'
> Missing period.
>
>> +      (setq code-list
>> +            (mapcar (lambda (s)
>> +                      (substring s 0 2))
>> +                    (delete "" (split-string status "\0"))))
> If you pass a non-nil OMIT-NULLS parameter to split-string, the
> (delete ""...) should become unnecessary.
>
>> +      (vc-git--git-status-to-vc-state code-list))))
> I would suggest dropping the temporary code-list variable here, and
> just do
>
>        (vc-git--git-status-to-vc-state
>         (mapcar (lambda (s) (substring s 0 2))
>                 (split-string status "\0" t)))