Staticman v2 provider configuration inconsistency

[travisdowns]

Environment

• Minimal Mistakes version: 4.18.1
• Ruby gem or remote theme version: jekyll-remote-theme (0.4.1)
• Jekyll version: 3.8.5
• Git repository URL: http://github.com/travisdowns/mm-test
• Operating system: Linux

Expected behavior

The staticman configuration implementation and doc seems a bit inconsistent. The configuration doc shows that the branch key should be nested under comments, i.e., at site.comments.staticman.branch:

However, in the template _config.yml in master, staticman is not nested under comments, but at the top level:

I believe this is the way it was supposed to work for staticman v1, because the staticman API bridge lookins in _config.yml for it's own configuration parameters.

However, for v2, this stuff goes in a separate staticman.yml file. So the actual bug I find is that in the staticman_v2 provider, it is checking the not-nested site.staticman.branch, rather than site.comments.staticman.branch:

This means if you configure staticman v2, it will be missing the javascript associated with form submission. As it turns out, comment submission will still go through, but you end up getting redirected to the JSON results page of the API bridge, because the default action is to do that, when the JS is missing, I guess.

Suggested fix:

1. Change site.staticman.branch to site.comments.staticman.branch in the v2 provider.
2. Clarify the documentation and default config.yml where to put the staticman stuff depending on the version. Include nested under comments the v2 configuration for staticman.

Workaround:

To get the comments working in the meantime, you can just include a redundant staticman.branch key at the top level so the check in the v2 provider passes, which is what I'm doing:

comments:
  provider: "staticman_v2"
  staticman:
    branch: "master"
    endpoint: https://staticman-travisdownsio.herokuapp.com/v2/entry/

# redundant, needed only as workaround for provider_v2.html bug
staticman:
  branch: "master"

[mmistakes]

I would follow what is in the docs. I don't think the _config.yml example was ever updated when this bug was report before.

Related: #2351, #2043

[mmistakes]

And yes you're right, doesn't look like this was ever fixed in the provider include for staticman_v2 https://github.com/mmistakes/minimal-mistakes/blob/master/_includes/comments-providers/staticman.html#L1

And it should be:

{% if site.repository and site.comments.staticman.branch %}

[travisdowns]

@mmistakes - right, but it doesn't work if you follow the docs, although this clear only if you get half way through my bug report. Start from the text So the actual bug to see the actual bug.

Essentially, it's the same fix as #2351 but in a different file.

Edit: beat me by 4 seconds :)

[mmistakes]

The whole thing is a big ball of confusion anyways. v3 is really v2 with custom endpoints. In all honesty support for everything except the current custom endpoint method should be left. I don't think the public instance even works anyways and that would clear up all the issues related to that.

[travisdowns]

The whole thing is a big ball of confusion anyways. v3 is really v2 with custom endpoints. In all honesty support for everything except the current custom endpoint method should be left. I don't think the public instance even works anyways and that would clear up all the issues related to that.

Correct, although v2 and v3 are pretty similar. I.e., you don't need a different provider for them (v2 will work fine), the only difference really is in the endpoint property. I think this is more or less orthgonal since it involves confusion between v2/3 and v1.

I didn't follow what you meant by "... should be left", do you mean something like "should be removed"?

[mmistakes]

Should be left... keep support for it.
I deal with a lot of "my comments" don't work and they're almost always because the public Staticman endpoint is hammered and doesn't work.

Pointing people to that solution seems like a disservice since it almost never works anyways.

[travisdowns]

I agree, but I guess had troubling parsing this sentence:

In all honesty support for everything except the current custom endpoint method should be left.

I parse it as "the current custom endpoint method should be removed, everything else should be left".

By "custom endpoint" I assume you mean using your own endpoint, and not the public instances, so I parsed this as the opposite of what you meant (sorry... don't you get to meet the most annoying pedants on github?).

[mmistakes]

Yup 😄