Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Project Find and normal Find treat \s differently #218

Open
respectTheCode opened this issue May 21, 2014 · 73 comments
Open

Project Find and normal Find treat \s differently #218

respectTheCode opened this issue May 21, 2014 · 73 comments

Comments

@respectTheCode
Copy link

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
Copy link
Contributor

benogle commented May 23, 2014

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

@benogle benogle added the bug label May 23, 2014
@respectTheCode
Copy link
Author

Looks like this was already reported atom/scandal#5

@benogle
Copy link
Contributor

benogle commented May 24, 2014

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
Copy link

@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:

  • \n, \r, and their character code / unicode equivalents
  • \s
  • [^ (because you don't really wanna parse it, but any character set that's getting negated might match white space)

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
Copy link

+1

@filipesilva
Copy link

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

@brentonstrine
Copy link

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
Copy link
Contributor

benogle commented Feb 18, 2015

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
Copy link

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 benogle added the atom label Feb 18, 2015
@benogle
Copy link
Contributor

benogle commented Feb 18, 2015

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
Copy link

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
Copy link
Contributor

benogle commented Mar 2, 2015

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

@brentonstrine
Copy link

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

@benogle
Copy link
Contributor

benogle commented Mar 2, 2015

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
Copy link

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
Copy link

👍

1 similar comment
@rake5k
Copy link

rake5k commented Aug 17, 2015

👍

@fulldecent
Copy link

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

@eskhool
Copy link

eskhool commented Sep 15, 2015

+1

1 similar comment
@TadeasKriz
Copy link

+1

@Rosseyn
Copy link

Rosseyn commented Nov 14, 2015

Would love to see this get a bump in priority.

@dominic-martino
Copy link

+1 keep hitting this issue

@TristanSmithlib
Copy link

+1

@TristanSmithlib
Copy link

TristanSmithlib commented Oct 18, 2017

Work around: Forget Atom. Use your own script.

WARNING: Use with care, will overwrite your .git dir. Copy files into a safe scratch folder before using.

search-replace.py

#!/usr/bin/env python
import re
import os

regex = r"Foo\nBar"
replaceWith = r"Spam"

for dirName, subdirList, fileList in os.walk('.'):
    print('Found directory: %s' % dirName)
    for inputFilePathName in fileList:
        print('\t%s' % inputFilePathName)
        
        with open(inputFilePathName, 'r') as myfile:
            text = myfile.read()
        
        newText = re.sub(regex, replaceWith, text)
        
        with open(inputFilePathName, 'w') as myfile:
            myfile.write(newText)

@safareli
Copy link

So if you are searching for usage of SOMEVALUE using regex like\sSOMEVALUE\s you will miss matches which end with newline.

And it sucks!

And it's not an edge case!

Sublime text works well tho.

@safareli
Copy link

Also this is the most requested bug
screenshot 2017-10-20 17 05 34

@safareli
Copy link

safareli commented Oct 20, 2017

Also if it this issue was open in https://github.com/atom/atom it would bug with the most upvotes
(label:bug sort:reactions-+1-desc is:issue is:open first bug there has 26 upvotes)

@agraebe
Copy link

agraebe commented Nov 14, 2017

+1

@vemv
Copy link

vemv commented Nov 24, 2017

Until this is fixed, some sort of warning when using the multi-file search would be very useful.

Can we solve this UX issue please? It seems low-hanging fruit. Really opaque that something works in one mode but not in another, without any warning

@gwigz
Copy link

gwigz commented Feb 19, 2018

I was pulling hairs thinking my regex was bad, why is this a thing?

@thinkyhead
Copy link

thinkyhead commented Feb 25, 2018

why is this a thing?

I haven't had time to implement a substitute for this feature yet. I apologize, but I've been very busy.

@garrettw
Copy link

I found out last week that VS Code seemingly suffers from the same issue. So that made me feel a bit better about using Atom.

@thinkyhead
Copy link

I keep going back to Sublime Text for its wonderful text capabilities (alt-select, multi-line search, duplicate selected text, multi-paste, and blazing speed), but I must use PlatformIO, and Deviot is semi-broken, so have been trying Atom and VSCode. I give them both points for style and capabilities in many areas, but there are many reasons I was happy to pay $70 to the author of Sublime.

@jerbrisebois
Copy link

This is severely needed. I keep having to load up Sublime to accomplish this. I considered just making the full transition to Sublime but I don't think I'm there yet. But this made me consider it.

@margaretlhunt
Copy link

This seems to have been requested hundreds of times by now, but I'm seeing no movement. Will /n searching for project ever be available, @benogle? Our 40 person team at my company is considering switching editors, but this is definitely a make or break item. Everything else about Atom suits our needs, but we use mult-file multi-line find/replace regularly and this is a huge drawback that other editors do not have.

@PaulMorris
Copy link

+1 for fixing this or at least adding a warning or similar when regex includes newlines.

@black-snow
Copy link

@margaretlhunt Can't you just "fix" it and contribute if you're 40 people? Instead of buying some other product you could invest some time to make this feature possible and share it with the world (we'de be grateful).

@garrettw
Copy link

@black-snow You can't just assume things like that. From what she said, it is clear that they're currently using something that works for them at least somewhat well. It might therefore be hard to make a business case for all the extra work it would take to grok the Atom codebase and then make the needed changes. At the end of the day, gratitude from the FOSS community doesn't pay the bills.

@black-snow
Copy link

I cannot assume what, exactely? o.Ó
Bills don't pay themselfs just like bugs won't fix themselfs. I don't know anything about that team, that's why I ask. Some (hipster) company would launch a hackathon or something and if I was to convince my boss that Atom's better than XY because Z* buuuut there's this tiny thing that's messy - he'd say "then take a day and fix it". And we're keeping an eye on each and every penny :)

* Like "we'd be X % more productive and there'd be Y % less bugs because of Z".

@garrettw
Copy link

I don't know anything about that team, that's why I ask.

"Can't you just fix it?" does not come across as merely asking if they can fix it. Rather, it is equivalent to saying, "I think you should be able to fix it, so you should do that instead of complaining." -- which is a bit presumptuous on your part.

From your spelling, I would guess that English is not your first language, so maybe you just didn't understand the implication in what you originally said. In any case, I'm not going to argue about this here.

@brentonstrine
Copy link

I still don't understand why this has been set as a lower priority. I can't imagine what other features or bugs could possibly be more important than this--a severe missing feature since Atom was in beta! Unfortunately, I've mostly switched to VS Code at this point--it was only a matter of time that missing features like this became a big enough problem that new forks would emerge to keep moving forward. Very sad as I much prefer Atom.

@atom atom locked as off-topic and limited conversation to collaborators Mar 22, 2018
@lee-dohm
Copy link
Contributor

Thanks everyone for the feedback.

I can understand your frustration on this. It is something that has bothered me occasionally over the years also. On the other hand, the reason why it hasn't been fixed is also probably the reason why nobody from the community has stepped up to contribute a fix. Fixing this would require:

  1. Rewriting or rearchitecting https://github.com/atom/scandal
  2. Rearchitecting the project results view
  3. Probably rearchitecting the entire data structure for storing project results

It isn't a simple fix. Additionally, while it is a complete incapability of covering a specific scenario, searching and/or replacing within a project with a search pattern that crosses a newline border (which generally requires using a regular expression), it isn't one that comes up constantly. This also reduces the priority of addressing it.

But since the discussion is simply going in circles and not bringing up anything that people don't already know at this point, I'm going to lock this issue for further comments. This issue is labeled triaged so it won't be marked stale and closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests