On Sat, Sep 27, 2008 at 08:35:03PM +0200, Johannes Sixt wrote:
>
> > +core.cygwinNativeStat::
>
> This name is *really* odd, for two reasons:
>
> - If I read "native" in connection with Windows, I would understand Windows's
> implementation as "native". Cygwin is not native - it's a bolted-on feature.
>
> - This name talks about the implementation, not about its effect.
>
> Perhaps a better name would be core.ignoreCygwinFSFeatures, and the
> description would only mention that setting this to true (the default) makes
> many operations much faster, but makes it impossible to use File System
> Features A and B and C in the repository. "If you need one of these features,
> set this to false."
>
> (And after writing above paragraphs I notice, that you actually really meant
> Windows's "native" stat; see how confusing the name is?)
It was Shawn's suggestion. I don't care much about the name as long as
it is explained in the documentation... Therefore, I accepted what Shawn
said without giving it any thought. Now, when you bring this name to my
attention, I believe core.useCygwinStat (in the opposite to the current
core.cygwinNativeStat) would be a better name. Your name is okay too,
but a bit too long for my taste and not specific enough (I suppose
Cygwin does many FS related tricks). Anyway, I don't have a strong
opinion here, so just whatever most people like is fine with me :)
>
> > +static inline void filetime_to_timespec(const FILETIME *ft, struct
> > timespec *ts) +{
> > + long long winTime = ((long long)ft->dwHighDateTime << 32) +
> > ft->dwLowDateTime; + winTime -= 116444736000000000LL; /* Windows to Unix
> > Epoch conversion */ + ts->tv_sec = (time_t)(winTime/10000000); /*
> > 100-nanosecond interval to seconds */ + ts->tv_nsec = (long)(winTime -
> > ts->tv_sec*10000000LL) * 100; /* nanoseconds */ +}
>
> Shorter lines in this function would be appreciated (and not just because my
> MUA can't deal with them ;).
I am sorry, I did not notice that the line got longer than 80 columns.
I will resent the patch once the issue with the name of the option is
resolved.
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
