This is the mail archive of the cygwin-patches@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: For the curious: Setup.exe char-> String patch


----- Original Message -----
From: "Robert Collins" <robert.collins@itdomain.com.au>
To: "Michael A Chase" <mchase@ix.netcom.com>; <cygwin-patches@cygwin.com>
Sent: Friday, February 01, 2002 18:45
Subject: RE: For the curious: Setup.exe char-> String patch

>> -----Original Message-----
>> From: Michael A Chase [mailto:mchase@ix.netcom.com]
>> . . .
>> String++.h:
>>
>> ### Do you want String::concat() and String::vconcat to be
>> public?  The few
>> places I see them being used could be ... String ("first") +
>> next + next ...
>> You lose a little efficiency by not calling String::concat,
>> but you make up
>> some of it by not having to call String::cstr_oneuse().
>
> HMMM, worth thinking about. Remeber that vconcat can only be used with
> char *'s, and we don't want them :}. (think unicode native storage).
> There are other lower lever mechanisms to optimise String, but as we
> aren't CPU bound, I'm not concerned at this point. One such example you
> could look at is the SGI Rope class template. (I've not looked at that
> but it's similar to what another project I'm on has been creating from
> scratch, a team member recently said "hey, this is similar :}". As for
> concat vs +, concat canonicalises paths, which is what broke Chuck's //
> path references (because / indicated a posix path to the code AFAIK). I
> don't think thats an appropriate use for String:: though, so wrote +,
> and used that. Also, canonicalisation IMO should occur at the
> io_stream::open and related calls, not at every manipulation: file path
> fragments shouldn't get canonicalised.

This sounds like a case of violent agreement.  I'll apply the big patch in a
separate directory and see where concat and vconcat are being used in hopes
of replacing them.

>> log.cc/log.h:
>>
>> ### If I understand the change, a log line may be either
>> timestamped or
>> babble.  So a line can't be timestamped and only go to setup.log.full.
>> Likewise all lines in setup.log must be timestamped.  I think
>> we are losing
>> some useful capablities by changing from flags to level.
>
> mmm, yes, but we've also gained type safety. If you wish to submit a
> flags class ( I can enlarge on that if needed) to allow log
> (LOG_BABBLE|LOG_FULL & LOG_TIMESTAMP|LOG_LITERAL, String const &) that'd
> be fine by me. I like enforcing type safety where possible.

I'll have to do some thinking on that.  I've never tried to create a class.
Other changes would be more important at this point.  Would a LOG_PLAIN
(==LOG_TIMESTAMP) be acceptable to replace 0 in log() calls for now?  That
way it would be fairly easy to get back to the current log appearance if we
decide to.

>> mount.cc:
>>
>> ### It looks like it might be cleaner to make String cygpath
>> (String const
>> &) visible along with String cygpath (const char *, ...).  It
>> seems like
>> nearly every place I saw it used you are doing cygpath
>> (xxx.cstr_oneuse(),0).
>
> Yes, I want to... but doing it was going to be a right ol' pain
> initially, so I pu tit to the side.
>
>> ### The few places that involve concatenation could be handled by
>> String()+x+...  I'm willing to make a patch to catch any
>> leftovers so String
>> cygpath( const char *, ...) could be dropped.
>
> Please do.

I'll start on a patch to go over your big one.




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]