From 5e37a27d1f1db98b880ceaad2f25e864f0a80126 Mon Sep 17 00:00:00 2001
From: Matias Chomicki <matyax@gmail.com>
Date: Mon, 9 Oct 2023 13:15:12 +0200
Subject: [PATCH] Loki Autocomplete: Add more context to comments about
 situations and completions (#76144)

* Loki completion: add more context to comments

* Improve grammar of comment

* Completions: expand on parser-offering part

* Update public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts

* Formatting
---
 .../monaco-completion-provider/completions.ts | 39 ++++++++++++++-----
 .../monaco-completion-provider/situation.ts   |  7 ++--
 2 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts
index ab7b829e41c..a18ca9fded5 100644
--- a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts
+++ b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts
@@ -356,26 +356,43 @@ export async function getLogfmtCompletions(
   otherLabels: string[],
   dataProvider: CompletionDataProvider
 ): Promise<Completion[]> {
-  if (trailingComma) {
-    // The user is typing a new label, so we remove the last comma
-    logQuery = trimEnd(logQuery, ', ');
-  }
-
   let completions: Completion[] = [];
 
+  if (trailingComma) {
+    // Remove the trailing comma, otherwise the sample query will fail.
+    logQuery = trimEnd(logQuery, ', ');
+  }
   const { extractedLabelKeys, hasJSON, hasLogfmt, hasPack } = await dataProvider.getParserAndLabelKeys(logQuery);
   const pipeOperations = getPipeOperationsCompletions('| ');
 
-  // {label="value"} | logfmt ^
+  /**
+   * The user is not in the process of writing another label, and has not specified 2 flags.
+   * The current grammar doesn't allow us to know which flags were used (by node name), so we consider flags = true
+   * when 2 have been used.
+   * For example:
+   * - {label="value"} | logfmt ^
+   * - {label="value"} | logfmt --strict ^
+   * - {label="value"} | logfmt --strict --keep-empty ^
+   */
   if (!trailingComma && !flags) {
     completions = [...LOGFMT_ARGUMENT_COMPLETIONS];
   }
-  // {label="value"} | logfmt --flag ^
-  // {label="value"} | logfmt label, label2 ^
+
+  /**
+   * If the user has no trailing comma and has a trailing space it can mean that they finished writing the logfmt
+   * part and want to move on, for example, with other parsers or pipe operations.
+   * For example:
+   * - {label="value"} | logfmt --flag ^
+   * - {label="value"} | logfmt label, label2 ^
+   */
   if (!trailingComma && trailingSpace) {
     /**
-     * Don't offer parsers: {label="value"} | logfmt ^
-     * Offer parsers: {label="value"} | logfmt label ^
+     * Don't offer parsers if there is no label argument: {label="value"} | logfmt ^
+     * The reason is that it would be unusual that they would want to use another parser just after logfmt, and
+     * more likely that they would want a flag, labels, or continue with pipe operations.
+     *
+     * Offer parsers with at least one label argument: {label="value"} | logfmt label ^
+     * The rationale here is to offer the same completions as getAfterSelectorCompletions().
      */
     const parserCompletions =
       otherLabels.length > 0
@@ -387,6 +404,8 @@ export async function getLogfmtCompletions(
   const labels = extractedLabelKeys.filter((label) => !otherLabels.includes(label));
 
   /**
+   * We want to decide whether to use a trailing comma or not based on the data we have of the current
+   * situation. In particular, the following scenarios will not lead to a trailing comma:
    * {label="value"} | logfmt ^
    * - trailingSpace: true, trailingComma: false, otherLabels: []
    * {label="value"} | logfmt lab^
diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.ts b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.ts
index 0a9c78e33d4..2c426c3bf48 100644
--- a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.ts
+++ b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.ts
@@ -483,10 +483,11 @@ function resolveLogfmtParser(_: SyntaxNode, text: string, cursorPosition: number
 
 function resolveTopLevel(node: SyntaxNode, text: string, pos: number): Situation | null {
   /**
-   * Top level examples:
+   * The following queries trigger resolveTopLevel().
    * - Empty query
-   * - {label="value"}
-   * - {label="value"} | parser
+   * - {label="value"} ^
+   * - {label="value"} | parser ^
+   * From here, we need to determine if the user is in a resolveLogOrLogRange() or simply at the root.
    */
   const logExprNode = walk(node, [
     ['lastChild', Expr],