redhat-documentation / vale-at-red-hat

Vale config files, styles, and docs to help individuals and teams roll out Vale
https://redhat-documentation.github.io/vale-at-red-hat/
MIT License
39 stars 59 forks source link

Update SequentialNumberedCallouts.yml to ignore lines with conditional blocks #827

Closed gaurav-nelson closed 2 months ago

gaurav-nelson commented 4 months ago

Modified the script to ignore lines with conditional blocks.

The current script gives a false positive for callouts with the same number inside the conditional directives (ifdef and ifndef).

github-actions[bot] commented 4 months ago

⚡️ Deploying PR Preview...

github-actions[bot] commented 4 months ago

Click here to review and test in web IDE: Contribute

aireilly commented 4 months ago

Looks good @gaurav-nelson Can you also update the version of the script in vale-at-red-hat/tengo-rule-scripts/SequentialNumberedCallouts.tengo

aireilly commented 4 months ago

Ok, this one is very complex. To do this properly, we would need to fully evaluate ifdef and ifndef blocks in the context of the callout sequence. This is IMO too complicated especially for an error that is a warning level error only.

Suggesting a more lightweight approach which basically does not run the rule if it detects ifdefs are in use in the file.

There were some syntax errors in your script, so I modified it as follows:

SequentialNumberedCallouts.tengo

/*
    Tengo Language
    Checks that callouts outside the listing block are sequential
    $ tengo SequentialNumberedCallouts.tengo <asciidoc_file_to_validate>
*/

fmt := import("fmt")
os := import("os")
text := import("text")

input := os.args()
scope := os.read_file(input[2])
matches := []

// clean out multi-line comments
scope = text.re_replace("(?s) *(\n////.*?////\n)", scope, "")
//add a newline, it might be missing
scope += "\n"

prev_num := 0
callout_regex := "^<(\\d+)>"
listingblock_delim_regex := "^-{4,}$"
if_regex := "^ifdef::|ifndef::"
if_conditions := false

for line in text.split(scope, "\n") {
  // trim trailing whitespace
  line = text.trim_space(line)

  // check if we're entering a conditional block
  if text.re_match(if_regex, line) {
    if_conditions = true
  }

  //reset count if we hit a listing block delimiter
  if text.re_match(listingblock_delim_regex, line) {
    prev_num = 0
  }

  //only count callouts if there are no ifdefs
  if !if_conditions {
    if text.re_match(callout_regex, line) {
        callout := text.re_find("<(\\d+)>", line)
        for key, value in callout {
          //trim angle brackets from string
          trimmed := callout[key][0]["text"]
          trimmed = text.trim_prefix(trimmed, "<")
          trimmed = text.trim_suffix(trimmed, ">")
          //cast string > int
          num := text.atoi(trimmed)
          //start counting
          if num != prev_num+1 {
            start := text.index(scope, line)
            matches = append(matches, {begin: start, end: start + len(line)})
          }
          prev_num = num
        }
      }
    }
  }

if len(matches) == 0 {
  fmt.println("Callouts are sequential")
} else {
  fmt.println(matches)
}

To verify, install tengo go get github.com/d5/tengo/cmd/tengo and run:

tengo tengo-rule-scripts/SequentialNumberedCallouts.tengo .vale/fixtures/AsciiDoc/SequentialNumberedCallouts/testinvalid.adoc
gaurav-nelson commented 4 months ago

I didn't check the script. I'll have a look later sometime.

aireilly commented 2 months ago

Closing this. Fix delivered in https://github.com/redhat-documentation/vale-at-red-hat/pull/827

thanks @gaurav-nelson :partying_face: