horizontal rule spacing

[thomas-forbes]

Fixes #1170

I can add more tests when I get the green light about adding this general functionality.

[pjkaufman]

Do you mind clarifying the need for the addition of frontmatter parsing? I assume it is because --- can be a horizontal rule, but I would like to double check.

[thomas-forbes]

Yes, without the frontmatter package the AST recognizes the --- in the frontmatter definition as thematic break but with the package it recognizes the --- as a part of the frontmatter. eg

---
prop: value
---

asdf
--- 
qwer

[pjkaufman]

By chance does that happen when the YAML is ignored for that rule? I forget what exactly is used as the ignore value, but I think it is ---\n---.

[thomas-forbes]

Sorry I don't understand the question. Are you asking whether we would get the same behavior if we ignored yaml for this rule? Is there a way to do that without the frontmatter package? I didn't know about that but that should work.

For context this is difference in the AST between when the frontmatter package is used:

Input:

---
prop: value
---

asdf
___ <!-- underscores because `---` directly below a p turns it into a h2 -->
qwer

Output:

// with package
{
  type: 'root',
  children: [
    { type: 'yaml', value: 'prop: value', position: [Object] },
    { type: 'paragraph', children: [Array], position: [Object] },
    { type: 'thematicBreak', position: [Object] },
    { type: 'paragraph', children: [Array], position: [Object] }
  ],
  position: {
    start: { line: 1, column: 1, offset: 0 },
    end: { line: 7, column: 5, offset: 34 }
  }
}

// without package
{
  type: 'root',
  children: [
    { type: 'thematicBreak', position: [Object] },
    {
      type: 'heading',
      depth: 2,
      children: [Array],
      position: [Object]
    },
    { type: 'paragraph', children: [Array], position: [Object] },
    { type: 'thematicBreak', position: [Object] },
    { type: 'paragraph', children: [Array], position: [Object] }
  ],
  position: {
    start: { line: 1, column: 1, offset: 0 },
    end: { line: 7, column: 5, offset: 34 }
  }
}

[pjkaufman]

Sorry I don't understand the question. Are you asking whether we would get the same behavior if we ignored yaml for this rule? Is there a way to do that without the frontmatter package? I didn't know about that but that should work.

There is for the Linter. It uses a field to determine what types are to be ignored for the rule. The ignore type for YAML is defined here. It would allow for changing the YAML to ---\n---. So the above input would need to start with that instead. So you would add that to the ignore list if it does indeed keep from getting recognized as a horizontal rule.

[pjkaufman]

I just did a quick check and it looks like it is still necessary to have the frontmatter parser present otherwise it will incorrectly parse things. I will have to take a closer look at this PR later this week and give more feedback then.

[pjkaufman]

So overall this looks pretty good. I want to thank you for taking the time to add this.

A couple of things to note about the PR:

• I mentioned a couple of UTs that I would expect to see on the PR
• I also added some comments around removing the use of toml in the parser as I don't believe it is indeed needed
• I also wanted to say that the wording for the new rule looks good. But all of the other changes to the localization file will need to be reverted as that does not seem to comport with the style that the Linter has been using up until now (I am not sure if something needs to be added to the Linter configs to help whatever editor you are using respect the Linter's eslint and other rules)

[pjkaufman]

We will want to add some corresponding tests that test how the hrs work in blockquotes and nested blockquotes to make sure that the blank line generation works as intended for those scenarios as well.

[thomas-forbes]

Is what I added what you're looking for?

[pjkaufman]

This is definitely close. With the blockquote scenario, I was more wondering about the following scenarios:
Assuming horizontal rules can be in a blockquote:

> Blockquote
>
> ---
> > Nested Blockquote
> Blockquote
> > Nested Blockquote

And the following edge cases (how does it handle blockquotes of the same level with an empty blockquote line between them):

> > Blockquote
> > 
> > ---
>
> > Nested Blockquote
> Blockquote
> > Nested Blockquote

Callout scenario:
> > Blockquote
> > 
> > ---
>
> > [!Info] Nested Callout
> > Info here
> Blockquote
> > Nested Blockquote

[thomas-forbes]

Ok I have made the changes you requested. Let me know if there is anything else you think needs to be changed

[pjkaufman]

Other than the two edge cases/scenarios I mentioned. This looks good.

[pjkaufman]

I think I am going to merge this even with the two edge cases not accounted for as they are pretty big edge cases that have come up in the past, but typically do not come up very frequently. So I think we are fine without them for now.