Re: StringNG merge?

On Mon, Nov 12, 2012 at 1:05 AM, Amos Jeffries <squid3_at_treenet.co.nz> wrote:
>
> I've run the maintenance script on this branch and it found quite a few bits
> of bad code formatting. Please apply the attached patch. It is all
> whitespace corrections except one bad include of squid.h and some #if
> protection macro name problems.

Thanks, merged.

[...]

>> it's similar to PERL's split() function in purpose, and it can be used
>> to create simple tokenizers.
>>
>
> It seems useful functionality by itself "But where?" is the big question.

Original aim was to have a simple multi-pass parser.
But after feedback, it's obvious it takes a too simplicistic approach,
and its role would be better handled by more-specific parsers,
supported by the Tokenizer class.
I've removed it.

>>>> +class StrList
>>>> +{
>>>> +public:
>>>> + StrList() { }
>>>
>>>
>>> This class is missing a description.
>>
>>
>> It's an intermediate step, its purpose is to provide a path for
>> disambiguating String-used-as-a-list by the various strList*
>> functions.
>> It can be omitted from the merge.

> Even a note to say that is useful to prevent us trying to use it for
> something before completion. Also, experiments should not be left in the
> code submitted for audit unless it is to be audited - it wastes everybodies
> time reading the diff. If you want it to be audited and committed fine - but
> then it cannot be skipped. Or are you saying you are happy with dropping it
> out of the branch?

I've documented its purpose and commented it out via #if 0.
The source- and header files will remain for the strList* functions.
If you don't mind I'd leave this bit of dead code in, otherwise it can
be removed.

> Asside: One of the things about merge is that it pulls in *all* of the
> revisions from the merged branch, or a sequence from N..M. Cherry-picking is
> going to be difficult on this project merge I think. The depends on how you
> were going to cherrypick it though. (cd trunk ; bzr merge src/SBuf*
> ../StringNG) is possible - I'm not sure that picks up the revision history
> properly, but I have not done it on a branch using the 2a format
> repositories yet. The other merge forms will be rather messy unless you
> revert the experimental bits like this and shelve them for later re-commit
> to the branch after the trunk merges.

I am willing to merge this by hand, in order to forget the
390-revisions, 5-years long branch revision history.

> Back to the audit...
>
> All over:
>
> * The HERE changes are in trunk. Please remove all the new code debugs()
> mentioning of what function name they are being called from.
> + SBuf.cc and SBufExtras.cc seems to be threaded with "MYNAME" and scope,
> the other files with hard-coded function names.

Cruft. This branch predates HERE.. Removing the bunch of them.

> SBufTokenizer.*
>
> * can we inline most of these methods? (as proper inline definitions rather
> than using .cci)

Sure. All one- and two-liners inlined.

> src/base/TextException.cc:
> * the xstrdup() crash protection change here is a bug fix. Please apply to
> trunk asap and separately.

Done.

--
    /kinkie