find --modify trim breaks other options like line numbers and highlighting

[KyleKolander]

Version Used:
0.5.0.0

Steps to Reproduce:
Create a new file SearchMe.test with the following content:

There is nothing
    on the first
 three lines, but
we FOUND IT on the fourth line
and
  FOUND IT on the sixth line.

Run the following commands to see the difference:

orang find --extension "test" --content "FOUND IT" --line-number
orang find --extension "test" --content "FOUND IT" --line-number --display trim-line
orang find --extension "test" --content "FOUND IT" --line-number --modify trim
orang find --extension "test" --content "FOUND IT" --line-number --modify trim --highlight match

Actual Behavior:
Line numbers and highlighted matches are removed when using --modify trim

Expected Behavior:
Using --modify trim should not break other functionality. Documentation says it should only remove whitespace, but it is much more destructive with the output. The --display option is deprecated and shows that message every time you use it, without an option to suppress the deprecation message (at least none that I could find).

[josefpihrt]

Hi,

thank you for your post.

I thought about this issue for a some time and I think that, based on your feedback, --modify is overcomplicated, it can have too much possible values and, as you pointed out, it is not clear what is actually being displayed when using this option.

I think that the best solution is to deprecate --modify and to introduce a new option/parameter (WIP #87) which will be simpler and thus better understandable by the user. The new option will have less possible values but more importantly it will be single list of values (matches) which can be modified/transformed according to provided values (similar to applying LINQ methods).

The comparison between "default" and --function (as it's called now, final name may be different) output:

default output:
file by file, each line with match is printed and match is highlighted:

--function output:
one list of values (matches) which contains all matches from all files. Specified functions are applied on this list and the result is displayed. This implies that some option (like --line-number) are not available.

Any feedback on the PR is welcome.

[KyleKolander]

Thanks for looking into this. I use this tool frequently.

Huge caveat is that I'm not familiar with your code base or any of the decision points along the way, so I can only speak to it generally as a software architect.

To me, this feels like an order of events problem. If it were me, this is the approach I would explore first. The --modify actions need to be applied before any "display settings", and then it should all just work. Famous last words, right! 😄

1. Search
2. Modify - trim, transform, etc.
3. Sort - sort-by, then-by
4. Display - line numbers, highlight, etc.

Frankly, I think you were heading down good paths with both the --modify and the --display ideas. Rather than view them as mutually exclusive (and deprecate one in favor of the other), I see them as complementary. The trick is in delineating the two. For example, trim modifies the search result, whereas --line-number does not - it is purely for display. That's the easy part.

Then you're faced with another ordering problem with the Modify step. For example, let's say the user wants to trim leading whitespace and wants to perform a replacement. Do you first perform the trim, and then the replacement, or vice versa? In some cases, the order may not matter, but sometimes it very much will (depending on the available modifications). The easiest approach here is to perform them in the order provided by the user.

Maybe something like this:

orang find --extension "test" --content "FOUND IT" --modify trim-start --modify replace-all "we" "I" --sort-by name --then-by length desc  --line-number ",5:#,##0" "#fce803" --highlight match "#03adfc"

• Each modification is its own --modify. There can be zero or more of them. Each has a primary action, and depending on that action, zero or more arguments. For example, the replace-all action requires two arguments; the first being the text to replace, and the second being the replacement text.
• There can only be one --sort-by with the first argument being the field, and the second being the direction (optional, defaults to ascending).
• There can be zero or more --then-by, but only if there is a --sort-by. The argument are the same as --sort-by. They are applied in the order provided on the command line.
• Unlike the others, there is no explicit --display option; instead each display option stands on its own. This is an opinionated, but pragmatic, approach to simplify/reduce typing.
• The --line-number display option can have zero or more arguments. The first is a C# format string (e.g., ",5:#,##0" means right align a 5 characters string using a thousands separator, etc.). The second is a hex color string.
• The --highlight` expects the first argument to designate what to highlight. The second argument is a hex color string. There could be other formatting arguments if warranted/requested.

You get a ton of flexibility out of this design, and it's easily understood what will happen. My opinion is clearly biased. ha ha

IIRC, you have options for distinct, aggregate, etc. I didn't cover that here because I wanted to keep it as simple as possible while still demonstrating how it solves the original problem, as well as others. Regardless, that is another class of operation, so it would have to be another number inserted into the order listed above. It's not a display, sort nor a modify operation; therefore, it stands on its own.

If I were to implement this, I would start with the command line parsing and representation of the options/arguments. Then I would ensure they were executed in the required order. Then I would do one option at a time and try to deliver value in small incremental chunks. Hope this helps! Like I said - I use this tool frequently, so great work on this!!!

Happy to talk through it more if you want. I enjoy problems like this.

[josefpihrt]

First of all, I'm really glad that you use this tool and that you bring the user experience.

Let me to add some comments to your command:

orang find --extension "test" --content "FOUND IT" --modify trim-start --modify replace-all "we" "I" --sort-by name --then-by length desc  --line-number ",5:#,##0" "#fce803" --highlight match "#03adfc"

tl;dr This command should be orang replace

find vs. replace

You are using find command to essentially executing replacements so it would be better to use orang replace. The replace command already gives you ability to accomplish what is proposed here.

Sorting

The current implementation of rendering of results is sequential. That means that rendering results in a different order would not be easy to implement. And the question is what is the use case of sorting results in a different order?

Line Number

The main problem is that --line-number is a flag so adding values to it would be a breaking change.
Nevertheless, I understand the idea of indenting the line number to have the results properly aligned.

Colors

Frankly, I don't think it's worth the effort, even though it would be nice to have ability to tweak the colors.
In case of someone would be really implementing it I would be in favor of specifying colors in some config file then to put it directly in the command but that is just my personal opinion.

[josefpihrt]

I think the really the most important question regarding modify is: What is the real usefulness? In other words: which options are really used by a user.

Over the time I found out that I use pretty much only distinct and sort, maybe sometimes group to see matched files grouped by matches.

Current State

modify offers several kinds of options:

1. modify a list of matches

• sort
• distinct
• except, intersect (allows you to compare results from 2 files)
• group
• remove-empty (this one is typical example of a feature that is easy to implement but turns out to be useless)
• remove-white-space (same as above and moreover it's not clear whether it removes matches that are empty or white-space or if it removes white-space from a match, former is right.)

2. modify match

• trim
• to-upper/lower

It's questionable what is the usefulness of modifying each match in the results.

And if you really need to do it you can do it with replace:

orang replace -c foo -r "$&" --modify to-upper --dry-run --line-number

3. other

• aggregate
• aggregate-only

These options are sometime confusing even to me. The original intent was to give a user ability to display results by file and then display modified list of results. I'm in favor of removing these options.

• ignore-case (can be inherited from --content)
• culture-invariant (can be inherited from --content)

Desired State

I would say that the desired state is to remove from modify all options that are: useless, confusing, incomprehensible. That means that following options would be kept:

• sort
• distinct
• group
• except, intersect (not sure, will anybody ever use it?)

Next step to makes things clearer is to always display just a list of matches when using --modify options. That means that modified results will not be highlighted or contain line number as it's not something that is applicable. It makes sense to highlight match and/or display line number when the results are displayed line by line but not when the result are displayed as a list of matches separated by newline.

From the compatibility perspective it's better to introduce a new option (like --function) and to deprecate modify then to change functionality of modify.

Use other tool to post-process results

If you use --content-mode value and --path-mode omit you will get just the list of matches (separated by newline). You can then use for example PowerShell to modify the results.

... | select -unique

Real Usage

I would love to know if you have some real use case that would not be covered by the proposed solution. If you post it here we can find some solution to it.