Re: [PATCH 2/3] vc: Check for installed version control tools before use

[Kai <kai willadsen gmail com>]

On 26 July 2010 05:46, Peter Tyser <ptyser gmail com> wrote:
> <snip>
>
>>> +
>>> +            # See if the necessary version control command exists.  If not,
>>> +            # make the version control choice non-selectable.
>>> +            if vc._vc.call(["which", avc.CMD]):
>>
>> I'm no Windows developer, but I'm guessing a call to 'which' isn't
>> portable. I'm not sure if there's a better option, but can we just try
>> to popen/call the command in question and catch OSError or something?
>
> I was under the impression that Cygwin was a requirement for meld
> under Windows, in which case I'd think 'which' would be installed.  We
> could use some other method such as the one you describe or something
> like: http://stackoverflow.com/questions/377017/test-if-executable-exists-in-python,
> but both are a bit more complicated than a basic 'which' call.

Yeah, I think you're right. I wish there was a better option, but I'm
okay with 'which' for this.

>>> +                self.combobox_vcs.get_model().append( \
>>> +                    [avc.NAME + " (" + avc.CMD + " Not Installed)", avc, False])
>>
>> This needs to be translated, and just concatenating isn't i18n
>> friendly anyway. Off the top of my head, how about something like:
>>    [avc.NAME + _("(%s not found)") % avc, avc, False])
>> ...which would also warrant a translator comment.
>
> Thanks for catching that.  What do you mean by "would also warrant a
> translator comment"?  I'm not too familiar with internationalization.

Me neither. :)

I believe that any comment string on the line directly preceding a
translation will be copied into the po files, to give context for the
translators. In this case, I think we just need comments to indicate
what the string formatting operators represent, etc.

See http://live.gnome.org/TranslationProject/DevGuidelines/Use%20comments

> I will change the strings to the following unless someone else advises
> otherwise:
>            if vc._vc.call(["which", avc.CMD]):
>                err_str = _(" (%s Not Installed)" % avc.CMD)
>            elif avc.valid_repo():
>                if (self.vc is not None and
>                     self.vc.__class__ == avc.__class__):
>                     default_active = idx
>            else:
>                err_str = _(" (Invalid Repository)")
>
>            self.combobox_vcs.get_model().append( \
>                    [avc.NAME + err_str, avc, len(err_str) == 0])

My bad - looking at this I realised that this also presumes word
order. So in the end, you'd have to translate the whole lot and avoid
the string concatenation all together. I'd just replace the first
element of the last list with _("%s (%s)") % (avc.NAME, err_str), and
remove the spaces and parentheses from the error strings themselves. I
*think* that should be okay.

cheers,
Kai