New feature: Save Find All menu content into a new Editor.

[MKadaner]

Summary

New feature added to the Find All menu:

• Key #F2# copies all found lines into a new editor and closes the menu.
• Key #Shift+F2# copies all lines matching the current filter into a new editor and closes the menu.

Checklist

• [X] I have followed the contributing guidelines.
• [ ] I have discussed this with project maintainers:
  If not checked, I accept that this work might be rejected in favor of a different great big ineffable plan.
  

Details

Thanks to @alabuzhev for the main implementation idea. Everything else was simple.

[HamRusTal]

I left a couple of irrelevant questions here earlier today and just deleted these. Sorry for the noise and likely false notifications. 🙄

[johnd0e]

Key #F2# copies all found lines into a new editor and closes the menu.

So the new editor content is kind of grep results?

[MKadaner]

@HamRusTal, no problems.

So the new editor content is kind of grep results?

Yes, exactly. And this defined the choice of the accelerator key. When I conceived this feature, I thought about it as of redirecting grep output to a file, so F2 (save) felt quite natural. Then, I realized that copying the found lines to an Editor makes much more sense in all respects. But I still think of the feature as of "saving" the found lines.

Few UX questions:

1. Is F2 still a good choice? Can you think of a more natural accelerator?
2. Should F2 save visible (filtered) lines, and Shift+F2 all found lines or vice versa?
   • Currently, F2 means "save all lines," and "Shift+F2 "save filtered lines."
   • I tend to think that the meaning should be reversed: F2 should mean "save what you see" (i.e., filtered), and Shift+F2 the alternative?
3. Should the new Editor be opened in Save-to-SaveAs mode, i.e., prompt for the file name on the first save? The filename is, of course, pre-populated.
   • Currently, F2 (in the new editor) silently saves with the pre-populated file name. To save with a different name, one has to use Shift+F2.
   • The new name is generated by inserting .Filtered (customizable) before file's extension. This is what I normally do when I redirect output from grep.
4. Should the new Editor be considered "dirty," i.e., ask to save on Esc?
   • Currently it is "dirty."
5. If we go with F2, does it make sense to add Ctrl+S accelerator to the existing Edit.SaveFile.lua macro?

[johnd0e]

Is F2 still a good choice? Can you think of a more natural accelerator?

F4, AltF4, ShiftF4

3. Yes

4. No

[MKadaner]

Thank you, @johnd0e. F4, of course! And Alt+F4 by association with the (right) Alt used to activate filtering in the menu. So:

• F4 copies all lines to the new editor.
• Alt+F4 copies filtered lines to the new editor.
• On the first save, the new editor will ask for the filename (default is, for example, Far.Filterd.map).
• The new editor may be closed without any confirmation and without saving.

I'll update the PR accordingly.

[MKadaner]

Here is the behavior I want. Before the content is saved for the first time, the new editor should be ephemeral, i.e.:

• Should keep the content in-memory only.
• Should have associated file name (for SaveAs).
• Should not touch the file with this name, even if it exists.
• Can be closed without saving or confirmation (if the user did not touch the content).
• Should not add the file name to history.
• If saving is requested (explicitly or because the content was changed), should use SaveAs with the predefined file name.

After the file was attempted to be saved, the ephemeral mode is dropped, and the editor should behave as normal.

Questions:

1. Is this behavior good or at least acceptable?
2. I tried to achieve this behavior with the existing flags (FFILEEDIT_DISABLEHISTORY, FFILEEDIT_SAVETOSAVEAS, FFILEEDIT_DELETEONLYFILEONCLOSE), and I think it is impossible. I propose to add a new flag, FFILEEDIT_EPHEMERAL. Any concerns?

[HamRusTal]

I propose to add a new flag, FFILEEDIT_EPHEMERAL. Any concerns?

If this way gets taken, shouldn't adding this flag be a separate PR preceding the “Save Find All …”?

[MKadaner]

I propose to add a new flag, FFILEEDIT_EPHEMERAL. Any concerns?

If this way gets taken, shouldn't adding this flag be a separate PR preceding the “Save Find All …”?

If the flag is added in a separate PR, in that PR there will be no way for reviewers to play with this flag. (I test / debug them together anyway.)

I suggest reviewing both the flag and the feature together. When everything is fine, I can split PR into two if you still think it a good idea.

[MKadaner]

Ugh! I believe I addressed all comments and achieved the desired behavior.

@alabuzhev, @HamRusTal, please review and let me know if you want me to split this PR and push FFILEEDIT_EPHEMERAL change separately.

[MKadaner]

@alabuzhev, gentle ping. Last week three days out of five I badly wanted this feature; almost was at the point to clone the repo on my work computer and make a wildcat build.

[MKadaner]

I am addressing a couple of reasonable SonarQube warnings. Please let me know if you want me to refactor the places where we have "more than 3 nested if|for|do|while|switch statements."

[alabuzhev]

Please let me know if you want me to refactor the places where we have "more than 3 nested if|for|do|while|switch statements."

Only if they were introduced in this PR and it makes sense to do so.

[MKadaner]

I removed exposing FFILEEDIT_EPHEMERAL and addressed a couple of SonarQube warning.

As for the too deep nesting warnings, I did increase nesting in two places, both of which already had more than three levels. I experimented with some meaningful refactoring, but it did not go well: too much global state.

In both cases, I can move the body of if (!m_Flags.Check(FFILEEDIT_EPHEMERAL)) conditional to new functions, thus technically not increasing cyclomatic complexity, but I think the result would become even worse. So, I decided to leave these places as is.

Please let me know if you want me to introduce meaningless functions. Otherwise, I'll squash the commits and call it a day.

I can (try to) refactor FileEditor::Init in a separate PR if you want me to. I hope the priority would not be too high. 😀

[sonarqubecloud[bot]]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[alabuzhev]

Thank you

[MKadaner]

Thank you!

[nightroman]

Hello All, thank you for the promising feature.

A flaw? As far as I understand this pattern is used for new file names with results: {original-base-name}.Filtered.{original-extension}. With FarColorer plugin, in some cases this leads to lots of Colorer errors in result files. E.g. found pieces of original XML file Test.xml are no longer valid XML in Test.Filtered.xml (no closing tags, etc.).

Maybe it would be better if the result file is named like *.txt (neutral txt Colorer scheme) or even {original-base-name}.{original-extension}.Filtered hoping that this does not find any Colorer scheme for .Filtered. // IMHO, *.txt way is better.

NB Technically, it's not even about FarColorer. Other editor tools bound to file extensions may be confused, too.

[MKadaner]

Ouch! When I implemented this behavior, I had .log files in mind, but of course, all structured file formats are going to be damaged. This is totally unacceptable. Corrupted .xml files look awful with Colorer.

Appending .Filtered.txt to the original file name is a simple and safe solution. The only disadvantage, at least from my perspective, is that the original extension is lost even for non-structured files, which means coloring may change dramatically between the original and the filtered file.

Unless somebody (@HamRusTal?) suggests a more user-friendly, even if more complex to implement, solution, I'll change the behavior to .Filtered.txt. Will try to do it during the coming weekend.

[HamRusTal]

a more user-friendly, even if more complex to implement, solution

I did not look into the source code, but since the current approach involves the English word Filtered I assume the implementation uses FAR's l10n mechanism with .lng files already (and if not, it definitely should). With that in mind, I suggest that it is the entire resulting file name template that is put to the language file, with the original base name (as {1}) and original extension (as {2}) separated, like this: {1}.{2}.Filtered.txt. This way, those few users who are unhappy with the default will be able to customize (via FarEng.lng.custom) the template to their liking, e.g. {1}_grep.{2}.

P.S. An example of how these {1} and {2} should be substituted can be seen e.g. with the MListFileSize and MListFileSizeStatus localized strings.

[MKadaner]

I assume the implementation uses FAR's l10n mechanism with .lng files

Yes, of course.

Thank you for the good idea and the pointer. Will do.

I am also considering the following solution:

• Two format strings (in .lng file): {0}.{1}.Filtered.txt and {0}.Filtered.{1}.
• A config string with the list of extensions which should be preserved (the second format string will be used), e.g., .txt;.log;.csv.

What do you think? Is it too cumbersome for the users?

[johnd0e]

@MKadaner A couple of ideas for improvement:

1. Output Line/Column
2. Highlight found occurrences

Also, currently editor now is positioned in non-optimal way (LeftPos=CurPos), so left part of editor is hidden.

[MKadaner]

@johnd0e

Output Line/Column

No, or at least based on a config. Column number does not make sense at all because only one line is copied to the new editor, even if it has multiple occurrences. Line number, maybe... In my scenarios line numbers are generally not needed and even undesirable, but I can see how they may be useful. A new config value is easy to add. The only consideration is that so far you (maintainers) were generally reluctant about adding new configs. Or maybe a different pair of hotkeys, like Ctrl+F4 and CtrlAlt+F4? Just let me know please.

Highlight found occurrences

Currently I position cursor exactly the same way it is positioned in the host editor when Ctr+Enter is pressed in Find All menu or when it is closed by Enter. The logic includes "selecting" the pattern. I expect (have always expected) that the found pattern would be highlighted, but it is not. I was going to look at this place later. So far, I moved the (duplicated) logic into a separate function, so it will be relatively easy to adjust.

Or do you mean to highlight all found patterns? Is it possible? Yes, of course, it should be. Again, please let me know.

editor now is positioned in non-optimal way

You put it very mildly. Yes, I noticed it. It is awful. I think this will be my next task, probably after fixing file name format.

[johnd0e]

Or maybe a different pair of hotkeys, like Ctrl+F4 and CtrlAlt+F4

Or adding Shift to original hotkeys. Just to clarify my intentions: the resulting file could be used for mass-editing of lines with found occurrences. (there is a macro for that https://forum.ru-board.com/topic.cgi?forum=5&topic=49572&start=2600#19)

And for this purpose "line:" prefix would be enough, just like grep -n. (BTW there is another related grep options, such as -C for adding context lines)

Or do you mean to highlight all found patterns? Is it possible?

Yes, I meant exactly that. It can be achieved with ECTL_ADDCOLOR.

[HamRusTal]

• A config string with the list of extensions which should be preserved (the second format string will be used), e.g., .txt;.log;.csv.

Although I see your motivation, to be honest, this is not the kind of data that is supposed to reside in a language file. Maybe a new far:config setting instead (subject to the usual scrutiny though)? Also, do we need the leading periods? Hmm, perhaps if only to be consistent with PATHEXT…

[HamRusTal]

editor now is positioned in non-optimal way (LeftPos=CurPos), so left part of editor is hidden

Sorry for the offtopic, but this is the issue I've been suffering from for a long time, obviously regardless of the new functionality. I have saving the editor position (per file) across sessions turned on, and very often (but not always) the re-opened editor is positioned in this infuriating manner…

P.S. This issue affects the EditBoxToEditor.lua script for me as well. :(

[alabuzhev]

• A config string with the list of extensions which should be preserved (the second format string will be used), e.g., .txt;.log;.csv.

Although I see your motivation, to be honest, this is not the kind of data that is supposed to reside in a language file. Maybe a new far:config setting instead (subject to the usual scrutiny though)? Also, do we need the leading periods? Hmm, perhaps if only to be consistent with PATHEXT…

To avoid new options, maybe an intermediate dialog on AltShiftF4 or something, prepopulated with the generated name, akin to ShiftF4 in panels, which the user can then invoke via a macro and change that name in any way they like?

[MKadaner]

Or adding Shift to original hotkeys.

Shift sounds good.

Yes, I meant exactly that. It can be achieved with ECTL_ADDCOLOR.

I can do that. A bit lower priority. I should fix the worse things first.

• A config string with the list of extensions which should be preserved (the second format string will be used), e.g., .txt;.log;.csv.

Although I see your motivation, to be honest, this is not the kind of data that is supposed to reside in a language file. Maybe a new far:config setting instead (subject to the usual scrutiny though)? Also, do we need the leading periods? Hmm, perhaps if only to be consistent with PATHEXT…

To avoid new options, maybe an intermediate dialog on AltShiftF4 or something, prepopulated with the generated name, akin to ShiftF4 in panels, which the user can then invoke via a macro and change that name in any way they like?

Yes, I meant far:config setting. I would really like to avoid the intermediate dialog (even if it can be handled with a macro). A dialog would break my workflow 🤣, and a macro... well, I do not like them. Is a new option really that bad?

[MKadaner]

@johnd0e, speaking about line numbers. what do you think of adding or not adding them depending on whether they are currently visible in the menu? Kinda WYSIWYG.

Also, what do you prefer, this:

6: Perfect: ⒍
28: Perfect: Twenty-eight
496: Perfect: CDXCVI

or this:

  6│ Perfect: ⒍
 28│ Perfect: Twenty-eight
496│ Perfect: CDXCVI

[HamRusTal]

adding or not adding them depending on whether they are currently visible in the menu?

This is smart IMHO

what do you prefer

What about a plain TAB character as a separator here? It is usually “long enough” to preserve alignment, is not visualized by default (so does not add noise), and is easy to remove/replace automatically.

[MKadaner]

Thanks. I'll go with visibility then. Great!

What about a plain TAB character as a separator here? It is usually “long enough” to preserve alignment, is not visualized by default (so does not add noise), and is easy to remove/replace automatically.

I do not agree with the “long enough” part. With Tab size set to 4 (very common setting) and 1000+ lines in the original file (not uncommon), the alignment will be off:

1   Blah
10  Blah
100 Blah!
1000    Blah... Ouch!

Furthermore, I would highly prefer to right-align line numbers with spaces (just like it is done in the menu itself). It's much easier to parse visually. If the width of the line numbers column depends on the max number (the widest number fits exactly, again like in the menu), the TAB character will be adding different number of spaces (visually) depending on the size of the original file. I think this would be strange.

With line numbers right aligned, we could use space character for separator. It would still be reasonably easy to manipulate line numbers with a regex (^\s*\d+\s), but I strongly prefer a visible character, so that line numbers and the actual content can be told apart at a glance.

Now, the │ character (BOX DRAWINGS LIGHT VERTICAL, U+2502) as a separator is out of question because it cannot be represented in some encodings. So, we are left with :

      1: Blah
     10: Blah
    100: Blah!
   1000: Blah... Ouch!
1000000: Blah, blah, blah, blah!

If there are no strong objections, I'll proceed with right-aligned, width-fit, colon separated line numbers.