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

RPC Multiple Search Queries #748

Closed
scholtzan opened this issue Jul 15, 2018 · 11 comments
Closed

RPC Multiple Search Queries #748

scholtzan opened this issue Jul 15, 2018 · 11 comments

Comments

@scholtzan
Copy link
Member

Multiple Search Queries

This is for discussing and collecting my thoughts on adding support for multiple search queries. xi-editor/xi-mac#179 shows a mockup of what the interface for this could look like.

Why?

This feature might be unusual since it is not supported by other editors, but I think it improves readability when working with more complex queries. For example, to investigate specific patterns in log files, one regex might get quite long and hard to understand:
(Connect to 192\.168\.\d{1,3}\.\d{1,3}|Connection to host 192\.168\.\d{1,3}\.\d{1,3} (failed|interrupted)|Reconnect to 192\.168\.\d{1,3}\.\d{1,3})

Additionally, occurrences of the different search queries will be highlighted in different colors (at first a limited number of different hightlight will be supported).

Intended Behaviour

The search queries can be added and removed dynamically. I think it would make sense to prevent deleting the last search field so that added search queries are considered as additional queries.

All the queries are considered to be linked by an OR function. This means they could also be written as one regular expression that combines the queries with OR. All the queries support options such as case-sensitivity or regular expressions.

RPC Changes

  • add_find {}: signals to the core that a new search query is added

    • ID of the new search query needs to be returned to the frontend:
      • Enhance find_status
        • Always contains a list of all search queries
        • Additional element considered as new search query
      • Custom RPC command: find_added { id: ... }
      • Synchronously return ID (least preferred option)
  • remove_find {id: id}: removes the search query with the provided ID

    • ID of the new search query needs to be returned to the frontend:
      • Enhance find_status
        • Always contains a list of all search queries
        • Missing element considered as being removed
      • Custom RPC command: find_removed { id: ... }
      • Synchronously return ID (least preferred option)
  • find {id: id, ...}: add search query ID

  • find_status {[{id, ...}, {}]}: add search query IDs

Open Questions

  • Use selection for find: which find should be changed?

    • My current preference: additional search queries get removed, the first search query is set to the selection
    • Alternatives: preserve additional queries, set last used to selection, ...
  • Replace: Which occurrences should be replaced?

    • My current preference: replace all occurrences
    • Alternatives: no replace, or have a per-query replace text entry
  • Colorization

  • Keep support for old RPC?

@cmyr
Copy link
Member

cmyr commented Jul 16, 2018

Would it be possible to keep the same current "find" RPC, but to have it's argument be a Vec of queries? Then adding/removing queries is just a matter of sending the existing RPC with whichever queries we want to use; in core we can be efficient about checking whether queries are new.

An implementation question: Given the following buffer:

Hello this is some text

How do we handle two queries, "is" and "this is"? That is, do we allow overlapping matches (essentially treating each query independently) or do we treat it more like a regex join, where one match (the longest?) wins?

Use selection for find:

I'm worried about the case where the user has a bunch of carefully constructed search queries that get accidentally blown away by 'use selection for find'. One alternative could be that if more than one search query exists, use selection for find creates a new query.

Replace: I agree, I think replace replaces all. Having no replace is also an option. I don't think per-query replace is worth the hassle.

@scholtzan
Copy link
Member Author

That might be possible. In this case, queries are identified by their position in the Vec, right? One thing that needs to be considered here is that the highlight color for the queries should stay the same even if a new query is added/a query is deleted.

Right now I would handle it as a regex join. I was considering that the first occurring match wins (eg. with queries "is some" and "some text", the first one would win).

That sounds reasonable.

@cmyr
Copy link
Member

cmyr commented Jul 16, 2018

Position could work, but so could just equality testing of the members of the query, e.g. the string and the various options?

re: join: to clarify, you imagine running each query separately, but then removing overlaps, preferring 'earlier' queries over 'later' ones? Or were you thinking of actually combining them into a single regex internally, and then extracting match groups?

@scholtzan
Copy link
Member Author

The queries run separately and would prefer earlier matches.

@scholtzan
Copy link
Member Author

I am currently working on the colorization of matches. I think, previously we decided to have reserved styles for the different highlights for now (just like find and selections are currently handled).

Currently, the different themes are provided by the syntect plugin. I was wondering, in order to support different highlight styles would it make sense to extend the themes and provide additional highlight styles? And is there a simple way to load modified/custom themes?
@cmyr

@cmyr
Copy link
Member

cmyr commented Jul 23, 2018

good point.

There is work on custom themes very close to ready in #675. However there's a general problem that the number of search queries is unbounded. Are we going to just set a cap at like 10 or something?

In any case in the name of just getting this working, I would pick some max number of colors, and then I would just hard code those into xi-mac for now. We can add this to themes at a later point, or come up with some other long-term solution.

@scholtzan
Copy link
Member Author

Okay. I'd say that a maximum number of queries would make sense.

@jansol
Copy link
Collaborator

jansol commented Aug 13, 2018

Idk, I think I'd be fine with one highlight colour that's shared across all concurrent queries. Any more would get confusing very quickly.

@cmyr
Copy link
Member

cmyr commented Aug 14, 2018

@jansol you can play around with this now on the appropriate xi-mac/xi-editor branches. There's definitely some largish design questions to be resolved, but I certainly think that the multiple colours is interesting!

@scholtzan
Copy link
Member Author

That might also be something that could be configured in the theme settings.

@jansol
Copy link
Collaborator

jansol commented Aug 20, 2018

If the colours are defined by the theme (and thus can be identical) then I have no issue with this.

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

3 participants