Add a way to customize the completion result #82
Conversation
Nexucis
force-pushed
the
feature/enrich-completion
branch
2 times, most recently
from
cbbbb51
to
c67931d
Compare
README.md
Outdated
| @@ -66,6 +66,47 @@ In case you would like to deactivate the linter and/or the autocompletion it's s | |||
| const promQL = new PromQLExtension().activateLinter(false).activateCompletion(false) // here the linter and the autocomplete are deactivated | |||
| ``` | |||
|
|
|||
| ### maxMetricsMetadata | |||
|
|
|||
| maxMetricsMetadata is the maximum limit of the number of metrics in Prometheus. Under this limit, it allows the completion to get the metadata of the metrics. | |||
There was a problem hiding this comment.
People may confuse this with autocompletion limits for metric names themselves, how about:
| maxMetricsMetadata is the maximum limit of the number of metrics in Prometheus. Under this limit, it allows the completion to get the metadata of the metrics. | |
| `maxMetricsMetadata` is the maximum number of metrics in Prometheus for which metadata is fetched. If the number of metrics exceeds this limit, no metric metadata is fetched at all. |
There was a problem hiding this comment.
thanks for your correction !
src/lang-promql/complete/index.ts
Outdated
| httpErrorHandler?: (error: any) => void; | ||
| fetchFn?: FetchFn; | ||
| // When providing this custom PrometheusClient, the settings above will not be used. | ||
| prometheusClient?: PrometheusClient; |
There was a problem hiding this comment.
Doesn't have to be in this PR, but would it now make more sense to package the url ... fetchFn fields into their own PrometheusConfig, and then just define the remote field as: remote: PrometheusConfig | PrometheusClient? Or something like that. Then TypeScript will prevent invalid configuration combinations and things would be clearer.
There was a problem hiding this comment.
I won't mind to do it, I would just be sure, it won't be annoying to use. I mean if I want to change just the url, then I have to create the struct:
{
remote: {
PrometheusConfig: {
url: 'http://prometheus.land'
}
}
}Don't know if it will bother developer to have so much different interface, just to configure an url.
What do you think ?
There was a problem hiding this comment.
The middle step would be left out if you do it like I suggest above:
{
remote: {
url: 'http://prometheus.land'
}
}TypeScript would automatically notice that I'm now passing in something that satisfies the interface PrometheusConfig, not PrometheusClient. Of course then the url field should be marked mandatory in PrometheusConfig.
There was a problem hiding this comment.
ah yeah ok, I see what you meant. Nice thanks, will do that
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
Nexucis
force-pushed
the
feature/enrich-completion
branch
from
c67931d
to
a29d766
Compare
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
Co-authored-by: Julius Volz <julius.volz@gmail.com> Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
Nexucis
force-pushed
the
feature/enrich-completion
branch
from
5c00034
to
f7026da
Compare
|
@juliusv many thanks for your review. I think I changes everything you asked. Tell me if you are ok know with the changes |
|
👍 Thanks! |
This PR provides a way to inject a custom completion. It won't override the current completion but will enrich it instead.
It will be possible to trigger the custom completion for a certain kind of completion (like only when the system will autocomplete the aggregation)
related to prometheus/prometheus#8174