[cli] Backticks in commit message cause failure

[jklmli]

Version: 2.12.1

When running yarn validate-commit-msg "$(git log -1 --pretty=%B)", backticks are executed since they're interpolated into the argument.

Failure: https://circleci.com/gh/jiaweihli/monapt/53

[stevemao]

where are the backticks? what does "$(git log -1 --pretty=%B)" expand as?

[jklmli]

Copying the full commit message from the CircleCI link:

fix: use generic self-types

`getOrElse` and `orElse` use self-types in order to support typed upper bounds.[0]

In TypeScript 2.4, generic functions were checked more strictly[1].
This causes the implicit downward type cast to fail, so we explicitly invoke the cast in the method body.
This workaround is backwards-compatible with TypeScript 2.3.

Bounded polymorphism has been implemented[2], but true F-bounded polymorphism hasn't been.
This means a type like `interface Option<A, B = Option<A, B>>` is invalid.

Alternatively, we can solve this with a lower type bound, but these don't work against concrete classes[3].

---

We should also upgrade monapt's TypeScript dependency to 2.4, but there are unrelated errors compiling the tests.

[0] https://github.com/Microsoft/TypeScript/issues/13337
[1] https://github.com/Microsoft/TypeScript/wiki/What%27s-new-in-TypeScript#stricter-checking-for-generic-functions
[2] https://github.com/Microsoft/TypeScript/wiki/What%27s-new-in-TypeScript#type-parameters-as-constraints
[3] https://github.com/Microsoft/TypeScript/issues/14520

This happens whenever there are any backticks, position isn't a factor.

[Garbee]

PR #99 adds a test internally for this. No internal code changes are needed for backticks to function correctly in parsing since they are contained within a string.

This seems like a problem with the shell interpreting the backticks as it is parsed. So you'd need to escape the commit message before handing it to the validator.

[jklmli]

I see...have you considered using a different interface for the CLI to avoid this issue? Rather than passing in the commit message, you could pass in a git reference and default to HEAD.

If this isn't viable, maybe the documentation should be updated to note the caveat re: escaping?

[Garbee]

I see...have you considered using a different interface for the CLI to avoid this issue?

What would a "different interface" be? Could you elaborate on this a bit with some details about what you're thinking could solve the issue internally.

Rather than passing in the commit message, you could pass in a git reference and default to HEAD.

Handling git in node isn't easy, especially to get cross-platform support done well. By asking that the pure message be passed it makes what the package needs to do much simpler and direct. Therefore, far easier to manage.

If this isn't viable, maybe the documentation should be updated to note the caveat re: escaping?

Yup, working on this shortly now that I have a working example.

For now though this works in bash shell:

yarn validate "$(sed 's|`|\\\`|g' <<< "$(git log -1 --pretty=%B)")"

sample output

jonathan@dev-laptop:~/Code/slim$ yarn validate "$(sed 's|`|\\\`|g' <<< "$(git log -1 --pretty=%B)")"
yarn validate v0.27.5
warning package.json: No license field
$ validate-commit-msg "fix: use generic self-types
\`getOrElse\` and \`orElse\` use self-types in order to support typed upper bounds.[0]

In TypeScript 2.4, generic functions were checked more strictly[1].
This causes the implicit downward type cast to fail, so we explicitly invoke the cast in the method body.
This workaround is backwards-compatible with TypeScript 2.3.

Bounded polymorphism has been implemented[2], but true F-bounded polymorphism hasn't been.
This means a type like interface Option> is invalid.

Alternatively, we can solve this with a lower type bound, but these don't work against concrete classes[3].

---

We should also upgrade monapt's TypeScript dependency to 2.4, but there are unrelated errors compiling the tests.

[0] Microsoft/TypeScript#13337
[1] Microsoft/TypeScript/wiki/What%27s-new-in-TypeScript#stricter-checking-for-generic-functions
[2] Microsoft/TypeScript/wiki/What%27s-new-in-TypeScript#type-parameters-as-constraints
[3] Microsoft/TypeScript#14520"
Done in 0.20s.

[stevemao]

No internal code changes are needed for backticks to function correctly in parsing since they are contained within a string.

Agree. A side note: Maybe we could accept multiple inputs and concat them, and then treat the whole string as a input.

[Garbee]

Maybe we could accept multiple inputs and concat them, and then treat the whole string as a input.

My only fear here is we could end up validating an incorrect message fully. Since bash in particular uses spaces to denote different parts, the spaces (and newlines?) are stripped. So when you concatenate them after bash, you're merging without some spaces and we don't know where to put them back with accuracy.

[hutson]

This package has been deprecated. Please use https://github.com/marionebl/commitlint instead.