atom / find-and-replace

Find and replace in a single buffer and in the project
MIT License
242 stars 196 forks source link

Project Find and normal Find treat \s differently #218

Open respectTheCode opened 10 years ago

respectTheCode commented 10 years ago

In a normal Find \s matches a line break but in Project Find it does not.

file1

obj.method1().method2()

file2

obj
    .method1()
    .method2()

With a normal Find and RegEx turned on obj[\s]*\.method1 will find a match in both files, but a Project Find will only find file1

benogle commented 10 years ago

This is an issue with the way atom/scandal does searching. It breaks the files up into lines, and searches on each line.

respectTheCode commented 10 years ago

Looks like this was already reported https://github.com/atom/scandal/issues/5

benogle commented 10 years ago

I think it's reasonable to keep this open until fixed. If you have ideas on how to fix this, I'm open to them! I'm not sure of the best approach without being super memory inefficient. Currently scandal is very efficient, and I dont want to lose that for an edge case.

anandthakker commented 10 years ago

@benogle for whatever it's worth, I'd argue that this isn't an edge case: project search and replace with regexp's spanning multiple lines can go a pretty long way as a low-budget good-enough stand in for (much heavier/slower) semantic refactoring.

But that aside, I hear you on the efficiency issue. Dunno of this is really any help, but just in case: I think you could do a preemptive scan the regexp source for one of the following:

and if it didn't have any of these, you'd know that it couldn't possibly match across a newline, so the current, efficient scandal strategy would be safe.

felixakiragreen commented 9 years ago

+1

filipesilva commented 9 years ago

This seems to still be happening. My (silly/unfortunate) solution is to open some other editor and do the find/replace there.

brentonstrine commented 9 years ago

This is definitely not an edge case. Search and replace over multiple lines is a make-or-break functionality of a text editor. Somewhere in search you already have the "slower" functionality because searching for linebreaks works on a single file. I definitely think it's worth a quick scan (as @anandthakker suggests) to determine if multiple line searching is desired, and in those cases, switch to the same search that is already performed in a normal file find, then repeat it for each document. You could optimize by only performing the full search on documents that match some portion of the searched phrase using the normal efficient search.

benogle commented 9 years ago

Somewhere in search you already have the "slower" functionality because searching for linebreaks works on a single file.

The problem is with large files. Atom doesnt open files over 2mb right now, so a editor.getText().match(/someregex/) is totally fine. But what if you have a 1GB log file in your searched directory? I dont want to blindly read in the whole file then try to run a regex on it. So there needs to be more intelligence on multiline regexps. I dont know what that algorithm looks like though. e.g. If you search for .+\n+, and it hits a 1GB file, what happens?

brentonstrine commented 9 years ago

I guess it could try to open it, but that fails and you get a notice "The following files were skipped because they are too large:"

A more common example search might be something like </p>\n<h1> so it would start out by scanning the normal way for </p> and <h1>, though in your example search .+\n+ obviously most files are going to match to .+ and it won't be able to search \n+ the normal way, so it would have to open every single file, I guess. Is a slow search better than no search?

benogle commented 9 years ago

There is likely something clever we can do. Maybe something like you mentioned: analyzing the regex and matching newlines 'manually', + a max line match or something. It just isnt a simple project, and we've got quite a few things on our plate at the moment.

The following files were skipped because they are too large

I'd like to stay away from this if we can. We get enough flak for the 2mb limit ;).

brentonstrine commented 9 years ago

The following files were skipped because they are too large

I'd like to stay away from this if we can. We get enough flak for the 2mb limit ;).

I think limiting essential Atom features because of the 2mb limit will be cause for more flak, not less. I'm just imagining people saying something like "Not only does it have a 2mb limit, but because of that, Atom's search/replace is severely crippled."

Anyway, I don't want to get argumentative--sorry if it's feeling that way. I know that everyone knows the problem is solvable, it's just an issue of resources and priority. My vote is that this is a high priority issue that's worth dedicating some time to.

benogle commented 9 years ago

It's on our list, but unfortunately it is pretty low prio at this point.

brentonstrine commented 9 years ago

Yeah, no one assigned to it, no milestone set, even. Hopefully will be done before 1.0! Thanks.

benogle commented 9 years ago

The atom label adds it to our project management system, then we prioritize in Pivotal Tracker. When someone picks it up, it is assigned.

eddiemonge commented 9 years ago
a(
  href="#"
)

doing a normal find for ^\s+href="\#"\s+ finds the href line. Doing a global doesn't. Still single line search but the searches are different.

dbkaplun commented 9 years ago

:+1:

rake5k commented 9 years ago

:+1:

fulldecent commented 9 years ago

Is there a workaround I can use today for NON-regex project find and replace?

eskhool commented 9 years ago

+1

TadeasKriz commented 9 years ago

+1

Rosseyn commented 8 years ago

Would love to see this get a bump in priority.

M0unir commented 8 years ago

+1

gfpacheco commented 8 years ago

+1

agraebe commented 8 years ago

:+1:

black-snow commented 8 years ago

+500 - this is crucial

brentonstrine commented 8 years ago

What an embarrassment that this hasn't been given higher priority yet. A text editor that can't do multi-file search/replace... ridiculous.

dbkaplun commented 8 years ago

OMG @atom team prioritize find-and-replace usability improvements already!

bigxiang commented 8 years ago

Really want to see this feature :+1:

nlenkowski commented 8 years ago

+100 Just ran into this today and had to open up another editor to do a multiline find/replace across multiple files...

Mottie commented 8 years ago

This is an issue with the way atom/scandal does searching. It breaks the files up into lines, and searches on each line.

@benogle What if the files were broken up into say 500 character segments instead of line breaks?

benogle commented 8 years ago

@benogle What if the files were broken up into say 500 character segments instead of line breaks?

There would probably be issues if the regex spanned that boundary. It would probably be really unpredictable. While it's not ideal, the current version is predictable.

eddiemonge commented 8 years ago

As I pointed out above, I don't think this is limited to multi-line but the way the lines are scanned is different

Regaddi commented 8 years ago

+1000

jaswrks commented 8 years ago

+1

eddiemonge commented 8 years ago

@stedmanrh comments like that are not conducive to getting this fixed. If you really felt that way you could either 1) not use this since you don't value the developers time 2) be proactive and actually help figure out why its this way, create a test that demonstrates it failing, and even submit a fix yourself.

You should always strive to be positive and supportive as that is more likely to motivate people to want to help.

lee-dohm commented 8 years ago

Thanks @eddiemonge. I went ahead and deleted the comment because I feel it violated the Code of Conduct.

jlblcc commented 8 years ago

:+1:

NikhilVerma commented 8 years ago

Just caught this error today, it's very frustrating because I simply thought that my regex wasn't good enough. But when I search in a file I knew it would occur lo and behold it works. Can we add some notification or warning that certain regex will not work across the project?

lewismoten commented 8 years ago

I'm running into the problem as well. The good news is that I can use other software as a workaround. However, it would be great if I could stick to one tool to do the job.

Nick-Gottschlich commented 8 years ago

Also ran into this problem, luckily sublime text has this functionality, so if you're looking for a quick work around just use the drag your folder into the sublime text sidebar, right click and select "find and replace".

dorian-marchal commented 8 years ago

+1

thinkyhead commented 7 years ago

Due to this limitation I've been trying to find an alternative plugin to replace this one. Are there any Atom plugins that will use command-line tools like grep, awk, or sed to do search-and-replace? Then the plugin can be less complex and these well-heeled tools can do all the heavy lifting.

It would also be great if the plugin would open all files with matches in the editor, do the replacement, and then not save them (until Save All). This makes it easier to Undo the changes.

So, ideally I'd like to have it work exactly like Sublime, at least as one option.

Time to research how to write plugins, methinks!

lee-dohm commented 7 years ago

@CJBrew your comment was deleted as a violation of the Atom Code of Conduct as it is insulting or derogatory. You may consider this an official warning.

CJYate commented 7 years ago

@lee-dohm I'm sorry for the noise and any perceived rudeness. Mine wasn't the only comment in this thread to express frustration at a key piece of missing functionality, and there's others above that haven't been removed.

Most of the argument seems to be that regex search iterated across files would be inefficient. This is a valuable feature, and it might be better we make it work first, then optimise later. Does anyone here have a working module?

koljonen commented 7 years ago

Until this is fixed, some sort of warning when using the multi-file search would be very useful. I'd suggest a multi-line search option that can be toggled for current-buffer searches but is disabled when using the multi-file search. Hundreds if not thousands of hours collectively wasted debugging perfectly cromulent regexes could be avoided. 🐰 🐱 :shipit:

eliericha commented 7 years ago

Sorry for the noise, but any news on this? I run into this issue and have to switch to another editor several times a week..

@benogle any chance this might get some priority any time soon?

Just checking, no pressure :)

henri-hulski commented 7 years ago

Also run in it today spending some time to find out what is wrong with my regex. Would be great to get a warning until it is fixed.

Hammster commented 7 years ago

Also had to download sublime and do it from there. :+1: for this feature when its implemented

laurel commented 7 years ago

+1

nicoszerman commented 7 years ago

What I am doing as a workaround is Find All in the directory, and then go to each file and do a Replace All in the file. Anyways, I would love to see this fixed.