Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Find/Replace should not care about whether it recognizes an extension #467

Closed
th3coop opened this issue Jul 31, 2015 · 12 comments
Closed

Find/Replace should not care about whether it recognizes an extension #467

th3coop opened this issue Jul 31, 2015 · 12 comments
Assignees
Milestone

Comments

@th3coop
Copy link
Member

th3coop commented Jul 31, 2015

  • Create a folder with a file in it that has no file extension and no shebang. Don't allow Komodo to identify it basically.
  • Perform a find and replace in files in this folder
  • Komodo will not allow you to perform any kind of find/replace on this file as if doesn't know what type it is and could be binary.

I have two recommended changes and I think both should be be implemented (don't worry, one is easy)

  1. Change the warning message to convey WHY we're doing this. This was originally done to prevent random text being replaced in a binary file. Fair enough. The message should say "unknown file: Replacing text in this file could corrupt in an unrecoverable manner"
  2. Allow the user to override. Leave all the warning, the scary yellow, the warngin message the big triangle "!" symbol, but all the user to check the box if they choose to continue with the replace.
  3. I lied, I have 3 recommendations on this. Add a button to create a file association for that file type if possible (if there is an extension) or explain that creating a file association can prevent that warning from occurring.

I actually found a reference to this from Komodo 4 where a user was asking for suggestion 2.

Dev note:

We need to ensure that find/replace checks for binary vs textual files.

@th3coop th3coop added this to the 9.3 milestone Jul 31, 2015
@Defman21
Copy link
Contributor

I lied

How dare you do this!

@Naatan
Copy link
Member

Naatan commented Jul 31, 2015

I'm not sure i understand the reproduction steps.

I did this:

mkdir /tmp/foo
cd /tmp/foo
touch bla
komodo bla

Komodo opened it as a text file and allowed me to find and replace.

I would suggest a 4th option though:

If Komodo is opening a file and is not sure whether its binary or not warn the user "This may be a binary file and editing it as text can damage your file, are you sure you want to open this file?", if they say yes just open the file and treat it as text, no further warnings.

Could combine that with option 3 ("yes, and add file assoctiation for .bla").

@th3coop
Copy link
Member Author

th3coop commented Jul 31, 2015

I'm not sure i understand the reproduction steps.

You probably have a file association set for no file extension? Not sure. Just create a file that Komodo can't identify. Using ".blah" would work.

If Komodo is opening a file and is not sure whether its binary

There is no file opening involved. This is the find and replace in files so it does a "blind" replace of all the files in a directory. It doesn't open the file. Unless by "opening" you just mean the path being displayed in the Confirm Replacement dialog?

@Naatan
Copy link
Member

Naatan commented Jul 31, 2015

Ooooh ok, I thought you were talking about the in-file replace dialog.

Yeah definitely like ideas 2 and 3, I would take it a step further though and group all of these results together so someone can take an action on multiple files of type X. Maybe that's what you meant already.

@th3coop
Copy link
Member Author

th3coop commented Jul 31, 2015

group all of these results together

Nice, yes this would be a great addition.

@th3coop th3coop modified the milestones: Backlog, 9.3 Oct 6, 2015
@th3coop th3coop self-assigned this Oct 6, 2015
@Naatan Naatan modified the milestones: Perpetual, Backlog Feb 27, 2017
@Naatan Naatan changed the title Allow override of **Unknown Lang** warning in Confirm Please dialog Find/Replace should not care about whether it recognizes an extension Feb 27, 2017
@th3coop th3coop modified the milestones: 11.1, Perpetual Aug 30, 2017
@th3coop
Copy link
Member Author

th3coop commented Aug 30, 2017

I really hate this issue. I'd really like to see it fixed.

@th3coop
Copy link
Member Author

th3coop commented Oct 12, 2017

Because this was driving me so much crazy I looked at it again and I found a really simple fix:

Index: C:\Users\cgcho\Projects\komodo\KomodoIDE\src\find\findlib2.py
index 136e6f6..a72c310 100644
--- a/src/find/findlib2.py
+++ b/src/find/findlib2.py
@@ -292,7 +292,7 @@ def replace(regex, repl, paths,
 
     """
     journal = None
-    grepper = grep(regex, paths, skip_unknown_lang_paths=True,
+    grepper = grep(regex, paths, skip_unknown_lang_paths=False,
                    skip_filesizes_larger_than=skip_filesizes_larger_than,
                    first_on_line=first_on_line,
                    includes=includes, excludes=excludes,

Thoughts?

@Naatan
Copy link
Member

Naatan commented Oct 13, 2017

If we do this then we need to make absolutely sure that the files this code are ran on are textual and not binary files. Before it could use the extension for this, if we remove this then this might become a problem.

Regardless this should stay on 11.1, we do not fully understand the implications and so should not put it in a maintenance release.

@th3coop
Copy link
Member Author

th3coop commented Oct 13, 2017

If we do this then we need to make absolutely sure that the files this code are ran on are textual and not binary files.

There is a review feature. Is it really up to us to cover for the user who chooses not to review the changes that they are making? We could make the review mandatory I guess.

@th3coop
Copy link
Member Author

th3coop commented Oct 13, 2017

Or we could do the original suggestion of fixing how the dialog handles "unknown" files and allows the user to still select them. Either way, i keep hitting this and I'd really really like it to be fixed, not before 11.1 but not later either.

@Naatan
Copy link
Member

Naatan commented Oct 13, 2017

I don't want users to have to review whether a file is binary or not, that would be silly.

@mitchell-as
Copy link
Contributor

A reasonable binary file test is to check the first 64KB for a '\0' byte (nul byte). If there are no '\0' bytes in the first 64KB, you can reasonably assume it is a text file.

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

No branches or pull requests

4 participants