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