Add ignorePatterns option to filter completions with name regex #169
Conversation
| document = server.workspace.get_document(params.text_document.uri) | ||
| jedi_script = jedi_utils.script(server.project, document) | ||
| jedi_lines = jedi_utils.line_column(params.position) | ||
| completions_jedi = jedi_script.complete(*jedi_lines) | ||
| if not ignore_patterns: | ||
| completions_jedi = (comp for comp in jedi_script.complete(*jedi_lines)) |
There was a problem hiding this comment.
I don't think this needs to be a special case, does it?
There was a problem hiding this comment.
I'm not strongly opposed to removing it, but I included this special case as an optimization. Maybe the readability trade-off isn't worth it, so I'm curious to hear your thoughts!
My thought is that most users won't configure ignore_patterns. Since some calls to jedi_script.complete may return many completions, I believe special casing the empty ignore_patterns allows me to avoid repeated calls to a filtering function in the generator comprehension on line 199 for most users (or at least for me).
There was a problem hiding this comment.
Well I also don't feel very strongly about this!
My instinct would be to avoid special cases unless they are needed, you're suggesting that perhaps the special case is needed, for performance reasons.
I guess if we really wanted to decide which is 'right' then we should do some profiling to figure out whether the filtering is actually meaningfully expensive - but honestly that sounds like much more trouble than just making a decision.
Maybe a one-line comment explaining the motivation? I think I had suspected that you believed that these cases needed to be different rather than that this was a deliberate optimisation. But this discussion right here is probably already more than this whole thing deserves!
Depending on your needs, you could pass the following regexs (non-exhaustive, I'm sure there are more potential patterns that I haven't thought of):
"^_{1,3}$|^_[^_].*$|^__.*(?<!__)$""^_{2,3}$|^__.*(?<!__)$""^__.*?__$""^_.*$"Should resolve #168 , @karthiknadig please let me know if you agree!