Re: setup

[Achim Gratz <Stromeko@nexgo.de>]

Corinna Vinschen writes:
> Ok, for once.  But please make sure that you split the commit into
> functional chunks next time it's so large.

Well, the original patch was a lot smaller and I didn't really expect
that I'd have to replace such a large chunk of the old code to make
things work correctly.  I've thought about it some more and I guess I
can split off the search implementation in fromcwd.  The meat of the
patch would still be quite large, though.

> And send it to this list, so
> code snippets can be referenced in the review.

Sure.

> A few more nits, mostly on style:
>
> - What I'm missing are code comments in do_local_ini and do_remote_ini
>   describing what the new methods do.  Yes, the old code didn't have a
>   lot of comments either, but it would be nice to have this better
>   explained for future reference.

I'll add some.

> - Do you plan to keep the DEBUG_FROMCWD bracketed code in there?  If so,
>   it should probably just use `#if DEBUG' as elsewhere.

I was using similar code from crypto.cc as a guide.  I can remove that
code as it's debugged now.  In the long run it'd be better to have
Log(DEBUG_...) calls that get optimized out when doing the final build
indtead of conditional compilation.

> - The call to yydebug=1 is almost invisible now.  Please revert that
>   to the old 
>
>     [empty line]
>     /*yydebug = 1; */
>     [empty line]

OK.

> - ini_decompress as a function name may be a bit misleading.  What
>   about decompress_ini_file instead?

OK.

> Otherwise it looks ok, especially splitting the code into the new
> functions ini_decompress/decompress_ini_file and check_ini_sig and
> removing IniParseFindVisitor is a blessing.

I'd have really liked to refactor local and remote as well as they are
almost exact copies of each other.  I didn't try if file:// URLs would
have worked, maybe they do.  If so, the search part for the remote init
would need to be lifted so that both code paths would just process a
list of URLs.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Terratec KOMPLEXER:
http://Synth.Stromeko.net/Downloads.html#KomplexerWaves