earshinov / vscode-filter-lines

Extension for Visual Studio Code allowing to quickly find all lines matching a string or regular expression
https://marketplace.visualstudio.com/items?itemName=earshinov.filter-lines
MIT License
13 stars 3 forks source link

Feature request: Add context #1

Closed majkinetor closed 4 years ago

majkinetor commented 4 years ago

Great plugin.

How about adding context (configurable or ad-hoc, lines before/after).

earshinov commented 4 years ago

Thanks for feedback! The context feature looks useful. However, wouldn't entering context ad-hoc be too annoying for a user? Like, one needs to enter text, then lines before, then lines after… I wouldn't like to invent a micro syntax for this, too. What do you think?

majkinetor commented 4 years ago

However, wouldn't entering context ad-hoc be too annoying for a user?

It wouldn't if its optional :) For example, selecting option "Filter with context" and then context could be first 4.3 <search>

Also, search results could be presented in regions such as:

> Line 1
> Line 2
...

or

context -1.1
context -1.2
> line 1
context 1.1
context 1.2
---------------
context -2.1
context -2.2
> line 2
context 2.1
context 2.2
earshinov commented 4 years ago

@majkinetor , The regions thingy looks too specific. grep -A / -B / -C doesn't do that.

I have implemented the "with Context" commands. If you want, I can upload a pre-release version of the extension here so that you can give it a try and share your feedback before I make the official release.

majkinetor commented 4 years ago

@majkinetor , The regions thingy looks too specific. grep -A / -B / -C doesn't do that.

How could it, console is not an editor. When you think about it more, its really perfect solution IMO - with everything folded you don't see a context (so its same as before) and you could unfold a context on lines that particularly interest you (so you eleminiate unneeded context noise of lines that do not interest you).

With that in place its easier to set context globally as it wont trouble you unless you need it.

Powershell command Select-String basically allows you to do that - context is extra property where you could show it only on particular line.

I have implemented the "with Context" commands. If you want, I can upload a pre-release version of the extension here so that you can give it a try and share your feedback before I make the official release.

Sure, bring it on.

majkinetor commented 4 years ago

Here is an example I was talking about:

PS> (Get-Process | Out-String) -split "`n" | Select-String vm -Context 3.3 | set res; $res

>       9     2,43      10,11       0,00    5008   0 vmcompute
>       0 1.907,85   1.904,06       0,00   39144   0 vmmem
>      25    46,80      32,33       0,00    2500   0 vmms
>      33    12,15      19,60       0,00   38632   0 vmwp

PS> $res[1].Context.PreContext; $res[1]; $res[1].Context.PostContext
      7     1,49       7,11       0,00   77108   0 unsecapp
    114   164,50     212,84     244,30    7132   1 Viber
      9     2,43      10,11       0,00    5008   0 vmcompute
>       0 1.907,85   1.904,06       0,00   39144   0 vmmem
     25    46,80      32,33       0,00    2500   0 vmms
     33    12,15      19,60       0,00   38632   0 vmwp
     25     7,03      21,50       0,00    4108   0 vpnagent
earshinov commented 4 years ago

Ah, sorry, I didn't realize that you were talking about folding ranges rather than marks in the text. There are several problems with folding ranges:

I've got an alternative proposal. VS Code knows how to fold plain text documents by indentation level. The extension could format matching lines with context as follows:

line 1
  context -1.1
  context -1.2
  line 1
  context 1.1
  context 1.2
line 2
  context -2.1
  context -2.2
  line 2
  context 2.1
  context 2.2

If you enable line numbers, folding ranges will work fine even if your original document has indented lines itself. In your example, filtering with indented context and line numbers would produce something like this (this excerpt is only for illustration, the extension formats line numbers slightly differently):

line numbers

2 line 1
  0 context -1.1
  1 context -1.2
  2 line 1
  3 context 1.1
  4 context 1.2
7 line 2
  5 context -2.1
  6 context -2.2
  7 line 2
  8 context 2.1
  9 context 2.2

It also should be fairly easy to implement. What do you think?

earshinov commented 4 years ago

filter-lines-0.2.0.zip

This is a pre-release version of the extension, with context printed out in "plain" form, without indentation. Rename this .zip into .vsix and use "Install from VSIX".

installing from VSIX

You can see the updated README here: https://github.com/earshinov/vscode-filter-lines/tree/dev#readme

majkinetor commented 4 years ago

I've got an alternative proposal. VS Code knows how to fold plain text documents by indentation level. The extension could format matching lines with context as follows:

That crossed my mind too and it would indeed be good enough instead of fighting whatever plugin API lets you do.

This is a pre-release version of the extension

I tested it for some time and didn't find any problem.

Now some nitpicking: I do think tho that we should perhaps have settings for default context that would set those values automatically in the first prompt when you use Context options. This is good for parity with other options (such as line numbers).

One thing that troubles me just a little bit is that we now have all those menu items and context is choosen somewhat arbitrary (why not line numbers for example) and is somewhat spammy. Perhaps better idea is to do one or all of the following:

  1. By default show only regular filter line options without context
  2. Enable context if I press modifier while selecting an option (for example shift).
  3. Let the one choose among options:
    • [x] Use modifier for selecting context
    • [ ] Add context menu items instead

This could also work for other settings: line numbers, case sensitivity and regex. In this way, one can configure the plugin to its desire - use spamy menu items, use modifiers or nothing at all but only defaults and have regular 3 menu items.

I am perfectly happy now that we have this, I am just thinking about UX loud, dont mind.

I am really gratefull you did this.

earshinov commented 4 years ago

I've got an alternative proposal. VS Code knows how to fold plain text documents by indentation level. The extension could format matching lines with context as follows:

That crossed my mind too and it would indeed be good enough instead of fighting whatever plugin API lets you do.

OK, I will implement it then.

Now some nitpicking: I do think tho that we should perhaps have settings for default context that would set those values automatically in the first prompt when you use Context options. This is good for parity with other options (such as line numbers).

Maybe it will make more sense to save the last value the user entered? It is done already, but only if you have "preserveSearch": true in settings, and only during the lifespan of the VS Code window (that is, the value will not persist if you close and re-open VS Code).

One thing that troubles me just a little bit is that we now have all those menu items and context is choosen somewhat arbitrary (why not line numbers for example) and is somewhat spammy

Yes, it concerns me as well. But I also wouldn't like to create a multi-step wizard as in, for example, the multi-replace extension:

step 1

step 2

step 3

To me, three steps already seem tedious.

I provided separate "with context" commands because I presume that a single user might want to filter without context in some cases, and with context in others. So I wanted both commands to be accessible without editing settings. In contrast, I don't think that a user would want to switch between "with line numbers" and "without line numbers" as often. At least, I haven't received any requests about it so far.

Also, I don't think it hurts to have more commands in the command palette. VS Code remembers which commands one uses more frequently and suggests them first. Unused commands will end up at the bottom of the list and won't distract you,

Enable context if I press modifier while selecting an option (for example shift).

I'd rather leave it to the user. In contrast to command palette commands, keybindings can be easily defined by the user. I list all commands and their arguments in the README, so this should be fairly straight-forward.

{
    // "Filter Lines: Include Lines with String" that always uses context: 1
    "key": "shift-ctrl-k shift-ctrl-s",
    "command": "filterlines.promptFilterLines",
    "args": {
        "search_type": "string",
        "context": 1
    }
}

I am perfectly happy now that we have this, I am just thinking about UX loud, dont mind.

Thank you for you input đź‘Ť Please tell me what you think about my suggestions above. Do you think that custom keybindings are enough to make the extension comfortable to you?

majkinetor commented 4 years ago

All that is reasonable.

I still think you should add global context setting. While context is needed, I doubt it is changed on case by case basis. Didn't know about preserve search stuff although its volatility doesn't make it very useful. Context could be set to match global value by default so people could just double enter to get to the search (and if possible context should be selected so that you can just type to replace it). Such thing seems trivial to implement yet offers measurable productivity gain.

I will personally NEVER use non-contextual search once folding is implemented as I dont see any benefit in doing so, given that

  1. in folded mode it is the same as non context search
  2. you can quickly unfold all
  3. you can unfold only lines that interest you which is probably going to be used the most by majority of users

This means that extra question for context is only anoying but with double enter is not that much of a hassle.

I agree that multi-step quesstinare is tedious.

earshinov commented 4 years ago

OK, thanks. Here is a summary of what I am going to do:

  1. Implement two options: indentContext (true by default) and foldIndentedContext (true by default)
  2. Modify the "with Context" commands so that the context is prompted second to enable double-Enter
  3. Remember the last used context forever (need to check if there is API for that). Use the last used context by default (or "0" if there is no last used context available) to enable double-Enter.
earshinov commented 4 years ago

Here is an updated beta version in case you'd like to give it a try:

~filter-lines-0.2.0.zip~ sorry, wrong version

filter-lines-0.2.0.zip

majkinetor commented 4 years ago

Works like a charm. I like it so much.

earshinov commented 4 years ago

Cool. Please write back if you notice any problems. I am going to make a release in around 2 weeks.

earshinov commented 4 years ago

Released version 0.2.0.

majkinetor commented 4 years ago

Since I updated to 0.2 I can't execute anything? I get for any command

Command 'Filter Lines: Include Lines with String' resulted in an error (command 'filterlines.includeLinesWithString' not found)

It however works on fresh install on different computer. Is this some artifact of installing unreleased version ? Any tips on how to fix it ?

majkinetor commented 4 years ago

NVM, I fixed it by installing previous version, then new version (reinstall of existing version didn't work)...

earshinov commented 4 years ago

Next time you can try looking at Output > Extension Host when you run the command. These logs might give you some clues.

image

I don't know why it could happen…