thomvaill / log4brains

✍️ Log and publish your architecture decisions (ADR)
Apache License 2.0
1.13k stars 95 forks source link

HTML comments break parsing of "Status:", "Decider:", "Date:" & "Tags:" fields #62

Open Philzen opened 2 years ago

Philzen commented 2 years ago

Bug Report

Description

As soon as one starts filling out the "Deciders" or "Tags" section, any HTML comments start are showing up as such in the knowledge base (i.e. the <!-- optional --> comment).

Looking at the generated HTML, it transpires that:

Steps to Reproduce

  1. create an ADR
  2. Fill out either "Deciders:" or "Tags"
    Actually: just putting an additional space between [list everyone involved in the decision] and <!-- optional --> is enough to trigger the behaviour
  3. Deploy or preview the knowledge base → the former HTML comments become visible

Expected Behavior

HTML comments should be ignored in generated output.

Environment

Log4brains version: 1.0.0-beta.11
Node.js version: v17.3.0.
OS and its version: Manjaro linux / whatever gitlab.com uses
Browser information: Firefox 96.0.2

Possible Solution

The parser that processes the "Deciders:" and "Tags:" section needs to be changed to recognize <!-- content --> and ignore them for processing (while still making them appear in the generated HTML output).

Philzen commented 2 years ago

Just checked, the same problem applies to the Status: field. When any comment appears there, status is not correctly rendered and the ADR remains in status DRAFT, no matter what is actually stated as its status.

When this issue is touched, i may make sense to fix this alltogether, so i amended the issue title accordingly.

Philzen commented 2 years ago

HTML comments are a problem for the "Date:"-field too. When a comment is placed after the date, the sorting in the knowledge base is the same as if there was no "Date:" list entry at all.

tylerhubert commented 1 year ago

@Philzen I've opened up a PR to fix this back on Jan 19th. #97 The PR has been waiting for review since then though.