input is not displayed correctly (non-existing blank lines added)

[jaklan]

Current Behavior

When there's no blank line between body and footer (and the line is recognised as footer due to existing issue reference), commitlint wrongly displays the input and adds that blank line between them, which makes the lint messages pretty confusing (please look at the example below).

Expected Behavior

Displayed input should be exactly the same as provided input.

Affected packages

• cli
• core
• prompt
• config-angular

Possible Solution

Display the initial input with no modification at all.

Steps to Reproduce (for bugs)

❯ echo "test(general): test

some body
related to #1

Issues: #2" | npx commitlint

⧗   input: test(general): test

some body

related to #1

Issues: #2
✖   footer must have leading blank line [footer-leading-blank]

(looking at what is displayed - the blank line is there, but it's actually missing)

Context

Your Environment

Executable	Version
commitlint --version	@commitlint/cli@16.2.3
git --version	git version 2.32.0 (Apple Git-132)
node --version	v17.9.0

[escapedcat]

due to existing issue reference

Could you link the issue? Thanks!

Would you have time and motivation to look into this?

[jaklan]

@escapedcat due to existing issue reference - I mean the reference in the commit message: related to #1 😄

If you can point which module is responsible for generating the output I will try to find the issue in logic, so even if I don't do the changes myself (I would need to find some time to get familiar with your contributing rules etc.), I would report the suggested change(s) so you can just process it further.

[jaklan]

@escapedcat okay, that was actually quite quick:

commitlint/@commitlint/lint/src/lint.ts

Lines 191 to 196 in c63802d

	return {
	valid,
	errors,
	warnings,
	input: buildCommitMesage(parsed),
	};

commitlint/@commitlint/lint/src/commit-message.ts

Lines 7 to 18 in c63802d

	export const buildCommitMesage = ({
	header,
	body,
	footer,
	}: CommitMessageData): string => {
	let message = header;
	message = body ? `${message}\n\n${body}` : message;
	message = footer ? `${message}\n\n${footer}` : message;
	return message;
	};

What's the problem? Instead of returning input: message as in the previous if cases in the lint function, we are building the input with the buildCommitMesage, which creates the blank lines between message, body and footer automatically. I would need to understand better why this is actually done and if it's safe to replace the above with input: message or we should rather provide additional if clause.

[escapedcat]

Would need to check, but most important cases are covered by tests™️ . For a start you could just give it a try and see if any tests fail.

[jaklan]

@escapedcat quick JavaScript lesson, but have it 😉

PASS  @commitlint/lint/src/lint.test.ts

so simply there's a missing test for the above case.

[escapedcat]

Have a look if this related to #3120 or #896

[jaklan]

@escapedcat I did a quick test and replaced input: buildCommitMesage(parsed) with input: message and run tests:

 FAIL  @commitlint/lint/src/lint.test.ts
  ● returns original message with commit header, body and footer, parsing comments

    expect(received).toBe(expected) // Object.is equality

    - Expected  - 0
    + Received  + 2

      foo: bar/n/nFoo bar bizz buzz./n/nCloses #1
    +
    + # Some comment to ignore

      285 | 	);
      286 |
    > 287 | 	expect(report.input).toBe(expected);
          | 	                     ^
      288 | });
      289 |
      290 | test('passes for async rule', async () => {

      at Object.<anonymous> (@commitlint/lint/src/lint.test.ts:287:23)

So what's the issue - we use buildCommitMesage because we don't want to display comments in the lint command output - what we provide:

      foo: bar/n/nFoo bar bizz buzz./n/nCloses #1

      # Some comment to ignore

vs what we display:

      foo: bar/n/nFoo bar bizz buzz./n/nCloses #1

And unfortunately buildCommitMesage doesn't have proper logic to keep the original line breaks (which is pretty reasonable, as it works on parsed dict and not the original message, so it knows nothing about line breaks).

We have then 2 solutions:

1. simply display original message with all comments

   I guess ignoring comments was done on purpose not to clutter the terminal with redundant output, so that's probably not the way to go.

2. don't use buildCommitMesage to remove comments

   buildCommitMesage doesn't care about the line breaks in the original input, because the goal of that function is completely different - it's aimed to build the message from parsed. So if we actually want to remove comments from the original input, but not modify its original formatting - we should have a separate function for doing that.

[jaklan]

@escapedcat

#3120 is about some issue with footer-leading-blank rule logic used to calculate if there's blank line before footer or not, so imho it's completely not related.

#896 is about parsing - when we provide:

test: footer-leading-blank bug

hello
see issue #1234

we get:

  body: 'hello',
  footer: 'see issue #1234',

and for:

fix: correct minor typos in code

see the issue for details

on typos fixed.

Reviewed-by: Z
Refs #133

we get:

  body: 'see the issue for details\n\non typos fixed.\n\nReviewed-by: Z',
  footer: 'Refs #133',

So in general - that's a problem with parser not being able to properly identify what is body, and what is footer. So also not related to the above one, but to conventional-commits-parser, and affects not the input display, but the execution of defined rules.

[wtho]

@jaklan Yes, #896 is purely about the implementation in footer-leading-blank.ts, although a better parsing might help with that issue as well.