Hi, On Fri, Aug 7, 2015 at 6:03 AM, Drew Neil <[email protected]> wrote: > Would it make sense to break this patch in two? One patch could contain the > implementation for :cfdo and :lfdo. The other patch could contain the > implementation for :cdo and :ldo. I think that :cfdo is ready to merge. > Whereas :cdo seems to be a bit more controversial and perhaps needs more > work? >
I think the last patch is ready for inclusion into Vim. I have fixed all the reported issues. I am not sure what else needs to be fixed in the ":cdo" implementation. Regards, Yegappan > > On Wed, Jul 29, 2015 at 9:34 PM, Yegappan Lakshmanan <[email protected]> > wrote: >> >> Hi all, >> >> On Sun, Jul 26, 2015 at 12:22 PM, Yegappan Lakshmanan >> <[email protected]> wrote: >> > Hi, >> > >> > On Sun, Jul 26, 2015 at 4:53 AM, Drew Neil <[email protected]> >> > wrote: >> >> I agree with h_east that if you’re planning to run the :substitute >> >> command >> >> across multiple files, it makes sense to use: >> >> >> >> :cfdo %s/pattern/replacement/g >> >> >> >> and not: >> >> >> >> :cdo s/pattern/replacement/g >> >> >> > >> > Depending on the task, you can use either the ":cdo" or the ":cfdo" >> > commands. >> > >> >> I haven't see any replies so far. I am not sure whether we are >> agreeing to add both the >> commands or only the cfdo/lfdo commands? Do you guys think that only >> the cfdo/lfdo >> commands will be useful? >> >> Regards, >> Yegappan >> >> > >> > If you want to perform text search/replace across all the files in the >> > quickfix >> > list, then the ":cfdo" command with ":%s/.../g" is the correct option >> > (as it is >> > more optimal). >> > >> > This is similar to using the "g" argument to the ":substitute" command >> > to >> > replace all the matching text in a single line. And using the "%" range >> > to replace text in all the lines. If you want to replace multiple >> > matching >> > text across all the lines in a file, then you have to pass both "%" and >> > "g". >> > If you don't, then the text will not be correctly replaced. This is not >> > a >> > problem with the ":substitute" command. >> > >> >> >> >> I can also see myself wanting to use the :cdo command in combination >> >> with >> >> :normal for certain types of task. But I’ve come across another >> >> problem. >> >> Suppose that we have a text file containing these four lines of text: >> >> >> >> http://example.com >> >> http://example.org >> >> http://example.net >> >> http://example.com http://example.org http://example.net >> >> >> >> Now let’s say that we want to turn each occurrence of ‘http’ to >> >> ‘https’. (We >> >> could use the :substitute command here, but let me use this to >> >> demonstrate a >> >> problem with using :normal). We’ll use :vimgrep to populate the >> >> quickfix >> >> list with 6 matches: >> >> >> >> :vimgrep /http\zs:/g % >> >> >> >> Then we’ll insert the ’s’ character in front of the colon with this >> >> command: >> >> >> >> :cdo normal is >> >> >> >> The resulting text looks like this: >> >> >> >> https://example.com >> >> https://example.org >> >> https://example.net >> >> https://example.com httsp://example.org htstp://example.net >> >> >> >> In the last line, we get ‘https’, then ‘httsp’, then ‘htstp’. Not >> >> ideal! >> >> >> >> The problem here is that the quickfix list records line and column >> >> numbers. >> >> If characters are added or removed near the start of the line, the >> >> column >> >> numbers for later matches on that line will no longer line up with the >> >> match >> >> that created the original quickfix list entry. >> >> >> >> I’m not sure if this is a problem with the quickfix list, with :cdo, or >> >> with >> >> :normal. >> >> >> > >> > This is a problem with the quickfix list functionality. Currently when a >> > line >> > is added or removed, then the line numbers in the quickfix list entries >> > are updated. But when a line is modified, the column numbers in the >> > quickfix list entries are not updated. Refer to the qf_mark_adjust() >> > function. >> > >> > In the above example, you should use ":s/../g" instead of the ":normal" >> > command. >> > >> > Regards, >> > Yegappan >> > >> >> >> >> On Sat, Jul 25, 2015 at 6:22 PM, Yegappan Lakshmanan >> >> <[email protected]> >> >> wrote: >> >>> >> >>> Hi, >> >>> >> >>> On Sat, Jul 25, 2015 at 9:55 AM, h_east <[email protected]> wrote: >> >>> > Hi Yegappan and Bram >> >>> > >> >>> > 2015-7-25(Sat) 12:27:56 UTC+9 [email protected]: >> >>> >> >>> >> Hi Hirohito, >> >>> >> >> >>> >> On Fri, Jul 24, 2015 at 3:42 PM, h_east wrote: >> >>> >> > Hi Yegappan, Bram and List >> >>> >> > >> >>> >> >> >> > >> >>> >> >> >> > Thanks for testing the patch. I will send out an updated >> >>> >> >> >> > patch >> >>> >> >> >> > in a few days. >> >>> >> >> >> > Hopefully this time it will get included. This has been >> >>> >> >> >> > outstanding for more >> >>> >> >> >> > than two years. >> >>> >> >> >> > >> >>> >> >> >> >> >>> >> >> >> The updated patch (against vim 7.4.796) is attached. >> >>> >> >> > >> >>> >> >> > Thanks. So now it's ready to include, right? >> >>> >> >> > >> >>> >> >> >> >>> >> >> Yes. Of course :-) >> >>> >> > >> >>> >> > I confirmed this patch. >> >>> >> > >> >>> >> > I found unexpected behaviors. >> >>> >> > >> >>> >> >> >>> >> Thanks for testing the patch and sending the bug report. I am >> >>> >> attaching >> >>> >> an updated patch that fixes the two problems. Let me know if you >> >>> >> see >> >>> >> any >> >>> >> issues with this attached patch. >> >>> > >> >>> > I confirmed that reported problem have been fixed. >> >>> > Thank you for quickly fixes. >> >>> > >> >>> > I think it is better to discuss. >> >>> >> > This is my opnion. >> >>> >> > When the search pattern exists more in a row, I think :cdo/:ldo >> >>> >> > confuse to use. >> >>> >> > and the processing time tends to be long. >> >>> > >> >>> > Do you understand that the results of the following two commands are >> >>> > different, >> >>> > When the search pattern exists more in a row? >> >>> > >> >>> >> >>> The ":cdo" command executes the supplied command for every valid entry >> >>> in the quickfix list. It is upto the supplied command to perform the >> >>> appropriate >> >>> action for every entry. >> >>> >> >>> > >> >>> > (1) :cdo s/\<cmdidx\>/ex_&/g | update >> >>> > >> >>> >> >>> In this case, the supplied substitute command replaces all the >> >>> occurrences >> >>> of >> >>> cmdidx in the current line. >> >>> >> >>> > >> >>> > (2) :exec "cdo norm!iex_\<Esc>:w\<CR>" >> >>> > >> >>> >> >>> In this case, the supplied replaces only the first occurrence of >> >>> cmdidx. >> >>> This is >> >>> not a problem with the ":cdo" command. This is a problem with the user >> >>> supplied >> >>> command. >> >>> >> >>> > >> >>> > The (1) is processed all search pattern. >> >>> > But, The (2) is processed first search pattern in a row. >> >>> > >> >>> >> >>> This is the expected behavior as this is a problem with the user >> >>> supplied command. >> >>> >> >>> > >> >>> > ':cdo' is not necessary, When use only :substitute. >> >>> > >> >>> > When we use the ':cfdo' command such as ':cdo', Speed is also >> >>> > faster. >> >>> > >> >>> > :cfdo %s/\<cmdidx\>/ex_&/g | update >> >>> > >> >>> > So I propose to including patch only ':cfdo' and ':lfdo'. >> >>> > >> >>> > How do you think? >> >>> > >> >>> >> >>> No. In some cases the ":cdo/:ldo" commands are useful and in some >> >>> other cases ":cfdo/:lfdo" commands are useful. >> >>> >> >>> You are assuming that the ":cdo/:cfdo" commands will only be used >> >>> to perform substitutions and the results in the quickfix/location >> >>> lists >> >>> are >> >>> from a search command (e.g. vimgrep). This is not always the case. >> >>> You can populate the quickfix list with output from various tools >> >>> (e.g. cscope, tags, lid, global, build output, static analysis output, >> >>> etc.). >> >>> >> >>> Regards, >> >>> Yegappan >> >>> >> >>> >> > >> >>> >> > Case#1 >> >>> >> > How to reproduce: >> >>> >> > 1. cd to vim src dir. >> >>> >> > $ cd (Vim clone dir)/vim/src >> >>> >> > 2. Start Vim. (including this patch version Vim) >> >>> >> > $ vim -N -u NONE >> >>> >> > 3. Grep word "cmdidx" from source and header using vimgrep. >> >>> >> > :vimgrep "\<cmdidx\>" **/*.[ch] >> >>> >> > 4. Open quickfix window. >> >>> >> > :copen >> >>> >> > 5. Do :cdo command. (Intentionally forget the '| update') >> >>> >> > :cdo s/\<cmdidx\>/ex_&/g >> >>> >> > >> >>> >> > Expect behavior: >> >>> >> > - E37 occurs once. >> >>> >> > >> >>> >> > Actual behavior: >> >>> >> > - E37 occurs continuously. >> >>> >> > >> >>> >> > >> >>> >> > -------- >> >>> >> > Case#2 >> >>> >> > How to reproduce: >> >>> >> > 1~4. (Same abobe.) >> >>> >> > 5. Do :cdo command. (Intentionally forget the ":w\<CR>") >> >>> >> > :exec "cdo norm!iex_\<Esc>" >> >>> >> > >> >>> >> > Expect behavior: >> >>> >> > - E37 occurs once. >> >>> >> > >> >>> >> > Actual behavior: >> >>> >> > - E37 occurs continuously. >> >>> >> > >> >>> >> > And, When press Ctrl-C after the '-- More --' display, buffer.c >> >>> >> > was >> >>> >> > modified unexpectedly. >> >>> >> > >> >>> >> > [original buffer.c:4901] >> >>> >> > if (eap->cmdidx == CMD_unhide || eap->cmdidx == CMD_sunhide) >> >>> >> > >> >>> >> > [modified buffer.c:4901] >> >>> >> > if >> >>> >> > >> >>> >> > (eap->exexexexexexexexexexexexexexexexexexexexexexexex___________________ >> >>> >> > _____cmdidx == CMD_unhide || eap->cmdidx == CMD_sunhide) >> >>> >> > >> >>> >> > >> >>> >> > -------- >> >>> >> > This is my opnion. >> >>> >> > When the search pattern exists more in a row, I think :cdo/:ldo >> >>> >> > confuse to use. >> >>> >> > and the processing time tends to be long. >> >>> >> -- -- 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.
