On 18:48 Sat 22 Nov , Bram Moolenaar wrote: > > Marcin Szamotulski wrote: > > > 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. > > Right, the range comes before the command, thus we don't know what to > do with the range when first encountering it. > > So we basically have two alternatives: > 1. What you say: parse the range and store it in some intermediate form, > then parse the command and once we know the type of rnage we can > compute the actual range. > 2. Go over the range twice: Once to skip over it and find the command, > then a second time when we know the command, and thus how to compute > the actual range. > > I would argue the second solution is better. We do not need to come up > with an intermediate format, with new symbols and stuff. Also, we do > not need to store anything in the first round. The disadvantage is > having to parse twice, but that should be fast and we can use the same > code: A function that skips over the actual computation in the first > round. > > How about that?
Sounds good. I will go with the second solution. 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 vim_dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
signature.asc
Description: Digital signature
