Hello, Pavel! > This patch makes the handling of the 'type/' directive much better. It > also improve the quality of the code.
I appreciate your goals, but your patch is too hard for me to understand. I don't want to apply your patch without understanding it, especially when it adds a goto inside a huge function. Maybe you could split it into smaller, more obvious parts? By the way, if I hacked this function, I would first try to split processing of type/ into a separate function. regex_command() is too large. It even has two different variables "p". Ok, I've just separated processing of 'type/' into regex_check_type(). Sorry that you would have to adjust your patch, but at least it will have some chances to be applied now. > o It checks if the data in 'content_string' is valid and doesn't > preform any further checks, whether it matches a regex. I undestrand that content_string is empty if it could not be read. It is properly terminated by the current code. Please limit your changes for now to those fixing bugs and _significantly_ simplifying code. > o There was such check in the original, code but it wasn't handled > properly, so that it wouldn't be used at all. Sorry, I cannot find that place, so please send a separate patch. > o Almost all of the code is guarded by the 'asked_file' variable. I'd rather see the code restructured into smaller functions, than "guarded". Too many levels of braces are hard to read. > 1. Why this piece of code doesn't use the subshell ? Why should it? Especially when we are feeding data from a remote server into "file", bad things can happen if "file" dies. > 2. If it's going to live like that for the time being is it acceptable > at least to redirect the error output to the standard output so if > 'file' is missing the user doesn't get its prompt messed up. I don't think it should be redirected to stdout - it should be ignored of put to a separate file and then displayed. ext.c is a single caller of mc_doublepopen() now, so it should be easy. P.S. I just realized that spliting regex_check_type() breaks caching of the "file" output, and I don't have time to fix it today, so your help will be appreciated. -- Regards, Pavel Roskin _______________________________________________ Mc-devel mailing list [EMAIL PROTECTED] http://mail.gnome.org/mailman/listinfo/mc-devel