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



<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.

>> +                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.
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])

Best,
Peter


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]