New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider YAML vs datadir vs config vs non-string keys #4393

Closed
bep opened this Issue Feb 9, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@bep
Member

bep commented Feb 9, 2018

No description provided.

@bep bep added this to the v0.37 milestone Feb 9, 2018

bep added a commit that referenced this issue Feb 9, 2018

hugolib: Temp. disable some YAML data tests
They fail.

We will have to think about this.

See #4393
@bep

This comment has been minimized.

Member

bep commented Feb 9, 2018

@vassudanagunta let us handle this in this issue. I have disabled some YAML tests for now.

There are many arguments for keeping it as it is now (string keys all over).

The question is:

Do we break something? Or: Maybe we fix more than we break?

/cc @dmgawel

@vassudanagunta

This comment has been minimized.

Contributor

vassudanagunta commented Feb 9, 2018

Roger. Just please leverage the significant time I already put into #4379. Pretty please 😉 .

@bep

This comment has been minimized.

Member

bep commented Feb 9, 2018

@vassudanagunta #4379 looks like a bigger issue, and I don't have time to consider that (at least not now). Remember: I did not implement the front matter handling nor the data handling, and I don't think it's fair to ask me to dig myself into a "rabbit hole" (your words) dug by others.

But I will try to fix this particular issue. And my thoughts are this:

  • Front matter and /data are meant to be portable across formats (which was the reason why I closed my eyes and merged the related PR)
  • We even have commands to do conversion between them that kind of does not work because of this
  • People should be able to mix-and-match YAML and TOML etc. content
  • Theme author should be able to write portable templates
  • Having YAML keys as interface{} makes sorting random. The lexicographically sort you get with string keys have its own problems, but it is better than random.

So, here is how I suggest we do this:

  • Keep the implementation as is now (adjust tests)
  • Add a check for non-string keys and print a WARNING to the console.

I suspect this will fix plenty more than it breaks. If anyone knows of any dramatic downside to the above, speak out.

@vassudanagunta

This comment has been minimized.

Contributor

vassudanagunta commented Feb 9, 2018

I thought it was fair to ask you given that you are the current steward of Hugo's vision and evolution toward 1.0. 😀 And to be fair(er), I did most of the work of laying things out very clearly. All that was needed was a decision by people who know Hugo's vision and have been around long enough to know what its users expect (not me!).

And above, you did just that! You definitively answered one of the two main issues I raised #4379 with a clear vision. So thanks! ❤️

I will update the tests as well as #4379 so that when you and the other Hugo maintainers have time, you can take a look at the remaining issue (overlapping data). All that is needed is a decision, including the decision to punt, in which case just merge #4379 as-is as it codifies in tests Hugo's current behavior, guarding against unintentional changes as well as clarifying it for anyone who decides something should change.

bep added a commit that referenced this issue Feb 12, 2018

hugolib: Re-enable YAML data tests disabled in f554503
Also gave basic tests for JSON, YAML and TOML identical inputs and expected outputs, a step toward JSON, YAML and TOML equivalency (see #4393 (comment)).
@vassudanagunta

This comment has been minimized.

Contributor

vassudanagunta commented Feb 12, 2018

Alright I implemented everything above in #4402 except:

Add a check for non-string keys and print a WARNING to the console.

@bep you accepted my PR before I got to it (bedtime!). Should I assume that you decided to do it or should I continue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment