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

Implement system for dynamic snippets based on the user configuration #14

Open
MatejKastak opened this issue Aug 27, 2022 · 7 comments · May be fixed by #28
Open

Implement system for dynamic snippets based on the user configuration #14

MatejKastak opened this issue Aug 27, 2022 · 7 comments · May be fixed by #28
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed

Comments

@MatejKastak
Copy link
Member

Creating snippets that would satisfy every user is impossible. There are many combinations of meta values that they might prefer.

To try to resolve this issue, we can implement similar system to vscode-yara. This would have to be implemented in the python part of this project, to be compatible with other editors, not only vscode.

We should also extend the documentation with information about how to use this feature.

@MatejKastak MatejKastak added documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed labels Aug 27, 2022
@infosec-intern
Copy link

Hey Matej! This is a very cool project you've got going - I'm excited to see where it goes! I'd be happy to help you build out this feature and answer any questions you have about the vscode-yara implementation.

To hopefully make things easier, my feature is essentially just a CodeCompletionProvider built on top of a configuration. The main code is here. There may be a better way to dynamically build snippets, but this worked for my purposes.

@MatejKastak
Copy link
Member Author

Hey! That's great 👍. I like the way you designed this functionality and I think that people were are to it. So let's try to make it also available in YLS.

So, to drive this implementation, let's break it to some smaller wins and iteratively add support. I started by reading the code in your great vscode extension. To make this portable between different code editors, I suggest to re-implement this functionality into the Python code (LSP server). We can create a new module that will handle that.

  • ❓ Was this implemented such that when user had no custom configuration (default settings in the editor), the dynamic snippets were disabled? And we would provide only generic snippets?
  • ❓ Do you have some example configuration that you were using? It would be nice to start from a real use-case, that we can write tests for and move from that.
  • We can call the function that constructs dynamic snippets in YLS here. And create a new module for that.
  • We will probably need to extend the configuration in vscode extension contribution points here (we can maybe adopt the spec from your extension, defined here).
  • Other editors offer a way to set the configuration for LSP server by specifying a JSON object in their respective configuration.

@infosec-intern
Copy link

question Was this implemented such that when user had no custom configuration (default settings in the editor), the dynamic snippets were disabled? And we would provide only generic snippets?

There's a really simple default meta configuration of just one entry: KEY = VALUE which can be seen in this code block and this test

Do you have some example configuration that you were using? It would be nice to start from a real use-case, that we can write tests for and move from that.

There's a nonsense example in one of the tests here

input: {'c': 'third', 'a': 'first', 'd': 'fourth', 'b': 'second'}
output: meta:\n\tc = "third"\n\ta = "first"\n\td = "fourth"\n\tb = "second"

Personally, I keep things pretty simple for my own rules though and use the following setting to generate a metadata section

"yara.metaEntries": {
    "author": "",
    "date": "${CURRENT_YEAR}-${CURRENT_MONTH}-${CURRENT_DATE}",
    "hash": ""
}

For the metaEntries setting, I've always been bothered by it being a simple JSON object and not having any structure. I could never find a satisfactory structure though, so if you have the same thought maybe we can brainstorm some solutions

@MatejKastak MatejKastak linked a pull request Nov 15, 2022 that will close this issue
1 task
@MatejKastak
Copy link
Member Author

Great, thanks for the examples and clarification!

I had something similar prepared in #28. I will have to fix tests but the most core functionality should be reimplemented.

I will try to finish it in the upcoming days, it should be enough

@MatejKastak
Copy link
Member Author

I think, I have the implementation finished 1:1, and tests are finally resolved (#28). I am not sure how the resolving of completion items work in VsCode, but we don't need to resolve anything in this implementation. Is this something you use and want to keep ❓

I was playing with the implementation, do you use other (meta, strings, condition) snippets ❓ I find them not that useful, when you can just generate the whole rule.

I suggest that we keep RULE snippet and maybe meta one.

@infosec-intern
Copy link

infosec-intern commented Nov 23, 2022

Keeping just the rule and meta snippets makes sense to me. I'm not entirely sure how many people actually used the other ones, and I also only used the rule/meta snippets myself. I mostly just threw those in to have the option.

Resolution is great for things that don't need to be searched while typing, such as method documentation. Once a user pauses typing to evaluate the snippets, then VSCode resolves the remaining items, which is typically a much smaller list than when the user started typing, so I used it to limit the work my provider was doing. It's definitely not necessary, especially in a first implementation, but I found it nice to have

EDIT: I almost forgot why I did that. I used the resolution method specifically to speed up the initial completion item offering. I found lookups into the configuration and building the metadata snippet slower than I wanted them to be while typing. Creating an initial list of non-dynamic completion snippets and then resolving just the ones the user wanted sped up the process to a point I was happy with. That may not be a problem for your async python implementation though

@MatejKastak
Copy link
Member Author

Thanks for the explanation, let's start with a basic implementation. Feel free to create a new issue if you feel that the code completion is not fast enough after this feature is implemented.

I found lookups into the configuration and building the metadata snippet slower than I wanted them to be while typing. Creating an initial list of non-dynamic completion snippets and then resolving just the ones the user wanted sped up the process to a point I was happy with. That may not be a problem for your async python implementation though

If we identify that this is a problem, we can optimize later. The approach that we can use is to not fetch the configuration data every time the request is handled, but we only watch for the settings change. Each time the settings change we recompute the snippets accordingly and cache them. After that the code completion would just return static strings, which should be plenty fast.

I kept all strings and condition snippets. What do you think about this? Can we merge it like this?

Functionality preview:
dynamic_snippets.webm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed
Development

Successfully merging a pull request may close this issue.

2 participants