On 17:04 Wed 12 Nov , Bram Moolenaar wrote: > > Marcin Szamotulski wrote: > > > > I had a look at the count patch. I don't like the solution much. > > > > > > I think it would work better to add a flag similar to NOTADD that > > > specifies the kind of count: window number, buffer number, etc. That > > > way there is no need to pass information from where the counts are > > > parsed to where there is a switch on the command. Can use > > > compute_count() directly. First computing counts based on line numbers > > > and then redoing that for something else doesn't seem right. > > > > > > The code that computes the counts looks much too complicated. > > > > > > Please give the tests a name, we stopped using numbers recently. > > > > I slightly improved the patch, I added NOTLNR to command's flags and now > > the count is not computed twice. I also refactored the code so it's > > simpler and I think easier to follow. > > Thanks for the update. The code is still quite bulky. What I don't > like is first adding NOTLNR and then, when it is set, using a switch on > the specific command. There already is NOTADR, which has a similar > meaning but would now be a bit different. > > It's probably better to add a new field in the cmdname struct that > specifies the type of range. Like cmd_argt, but with a separate list of > values, e.g. ADDR_LNUM, ADDR_WINDOW, etc. > > Then where the special characters such as "$" are parsed, that flag can > be used to decide what the value is: Last line number, highest window > number, etc. That avoids having to pass the type around and then later > using it. > > There we can also check for errors. E.g. when the address is > "/pattern/" while the command takes a window number.
I think there is no way of getting rid of `addr_T` struct. The reason is that when the range is parsed we don't yet know what is the command, so we cannot use ADDR_LNUM, ADDR_WINDOW yet. You're right though that `NOTLNT` wasn't a good idea. What I suggest that we first parse the range into `addr_T` structs, without setting the `ea.line1` and `ea.line2`; after parsing the range we know which command runs (what is checked by `find_command` at this point and now we can set `ea.line1` and `ea.line2` since we will now which of `ADDR_LNUM`, ... should be used. It can be done using the function I wrote `compute_count`. At this place we will be able to handle errors as well. I will write a patch for that if you don't have any objections. Best regards, Marcin -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
signature.asc
Description: Digital signature
