Skip to content
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

Version 2.0.0 #562

Merged
merged 4 commits into from
Mar 9, 2015
Merged

Version 2.0.0 #562

merged 4 commits into from
Mar 9, 2015

Conversation

SamyPesse
Copy link
Member

Version 2.0.0

How to test it?

Uninstall GitBook 1.x.x:

$ npm uninstall gitbook -g

Install GitBook CLI:

$ npm install gitbook-cli -g

You can now build a book using the version 2.0.0 (this will download the latest alpha version of GitBook):

$ gitbook build ./mybook

Changes:

Plugins Compatibility

Related:

@znmeb
Copy link

znmeb commented Jan 23, 2015

I like LaTeX but I definitely think it's a "nice-to-have". On the GitBook website it'll be OK, but the dependencies it pulls in on Windows or Linux workstations are huge - on the order of a gigabyte minimum.

That said, every machine I use has those already (plus Pandoc and its dependencies!) so it won't impact me if you do support LaTeX.

@SamyPesse
Copy link
Member Author

@znmeb LaTeX will not need to be a dependency of GitBook, it will be needed only if the author is using latex files.

Supporting LaTeX is not the priority of this release, but it'll now be possible.

@SamyPesse SamyPesse self-assigned this Jan 23, 2015
@winniehell
Copy link

I made some inline corrections for the German texts (f121635).

@winniehell
Copy link

@SamyPesse no 🍻 today anymore 😉 💤

@winniehell
Copy link

@SamyPesse 👍 (a little bit impressed that you do a complete rewrite within one week)

@SamyPesse
Copy link
Member Author

Thanks, I know perfectly the gitbook structure, so it's easy to rebuild it and make it better.

Is "Einführung" right for "Introduction" (99d9d49#diff-d20357f2347472b13b032ff4c59f77e7R8) ?

@AaronO
Copy link

AaronO commented Feb 16, 2015

@SamyPesse It would be great, to avoid having to wrap chapters with {% raw %}...{% endraw %} for templates containing {{ and }} which is a very common pattern especially for technical books (django, mustache, nunjucks, even our own doc ...)

The ideal behaviour would be that if the expression to display (in the ``{{ ... }}) resolves to "nothing" then print it as it was {{ ... }}`. I have to see if `nunjucks` allows that.

The other option would be to skip them inside markdown code blocks ...

What do you think ?

@SamyPesse
Copy link
Member Author

@AaronO I think this is a bad idea, because:

  • You are focus on a very small usecase (probably less than 0.01%)
  • To solve this very small usecase, you are bringin more conditional complexity in the parsing pipeline, this will make it less easy to understand/reproduce; this will transform gitbook as a conditional mess.
  • Your solution can probably handle {{ .. }}, but you'll have to support all the other templating syntax, and it'll not be as easy, for example for a condition {% if book.newVersionAlpha %}, how do you decide to process this condition or to display it? what if newVersionAlpha is defined only in the config of the book?

{% raw %} is easy to use, use the same syntax as blocks.

I think who are focus on this, because you just commit a small change on the documentation and you are focusing on your tiny issue, without understanding the bad consequence on bringing that kind of complexity into the simple generation pipeline.

Another solution can be a configuration in the book.json to disable tempalting, but I think we should just stick with {% raw %} because the problem doesn't exist for 99,99% of the authors and it's not even a problem because the workaround is simple.

@AaronO
Copy link

AaronO commented Feb 16, 2015

@SamyPesse I disagree,

It's used in "formatting" url examples for API docs etc ... : http://help.gitbook.com/build/push.html#git-url.

{% raw %} blocks aren't very obvious to be honest, I think people will just be really puzzled as to why GitBook isn't displaying what they expect and won't know about or think of {% raw %}.

Books on Go programming, Django programming (web programming in general), api docs etc ... will be effected by this suggesting that they patch their pages with {% raw %} just doesn't seem very friend to me, it's something we broke for them for no good reason (to those people in that case, it adds no benefit while adding the inconvenience of having to patch).

And over that, that's the current behaviour right now : https://github.com/GitbookIO/gitbook/blob/master/lib/parse/include.js#L8.

This really shouldn't be complicated to fix and I think it makes it more obvious to see typos if the person is using variables. It doesn't at all have to be a big change in the pipeline, it should be two or three lines with no conditional, that's not going to turn GitBook into a conditional "mess" ...

The way I see it, there's only two possible reasons a person's {{ x }} expression would evaluate to "nothing" :

  • They made a typo, for example : {{ exmple }} vs {{ example }}.
  • They really want to display {{ example }} as part of a code snippet or whatever in their book

In both those cases displaying something blank is in my opinion useless. I'm more likely to notice an error of a misspelled variable if it still displays {{ exmple }}, so that's useful even when I'm actually wanting to use templating and when I actually don't want to that's good too.

What is there to lose and what's the cost ?

Disabling templating is extreme and not needed when this "fix" is both useful, simple and compatible with what's already out there.

@SamyPesse
Copy link
Member Author

With your solution, books about Django, Nunjucks,etc will still have issue, your solution will change nothing to the processing of {% if .. %}, {% for %}, {% include %}, .... Your solution will only handle case of people writing simple {{ ... }} expression without knowing about templating.
Basically if the author is writing something else between {% ... %}, we'll still have to use {% raw %}.

You believe your solution is easy, but you'll have to handle correctly the expression parsing, for example: {{ file.mtime|date("%d")|uppercase }} or something like {{ "Edited +"file.mtime|date("%d") if (file.mtime < now() - (10*3600)) else "No recently updated" }}.
If your interpolation in the processing pipeline doesn't handle perfectly all the parsing cases, you'll cause more damage that you're trying to fix.

You are considering an hack to solve an UX problem (that you're the only one to encounter yet). I think it's pre-matured. The version 1.x.x was full of hack of this type, I would prefer avoid going back this way.

Why not wait to get feedback from the authors about this? See how much users are having issue with the {% raw %} then setup a proper solution?

But instead of talking about this, you're sure of your solution, that it's clean, then just do a Pull-Request and we'll talk on it.

@winniehell
Copy link

@AaronO Would disabling template parsing for book content by default and having to explicitly enable it solve your issue? I can imagine that people that make use of the templating syntax refer to the documentation beforehand (other than people using {s).

@AaronO
Copy link

AaronO commented Mar 3, 2015

@SamyPesse Input versions should be validated:

~/git/gitbook version/2.0*
❯ gitbook versions:link local .

~/git/gitbook version/2.0*
❯ gitbook versions

/opt/boxen/nodenv/versions/v0.10.17/lib/node_modules/gitbook-cli/node_modules/semver/semver.js:271
    throw new TypeError('Invalid Version: ' + version);
          ^
TypeError: Invalid Version: local
    at new SemVer (/opt/boxen/nodenv/versions/v0.10.17/lib/node_modules/gitbook-cli/node_modules/semver/semver.js:271:11)
    at compare (/opt/boxen/nodenv/versions/v0.10.17/lib/node_modules/gitbook-cli/node_modules/semver/semver.js:424:10)
    at gte (/opt/boxen/nodenv/versions/v0.10.17/lib/node_modules/gitbook-cli/node_modules/semver/semver.js:473:10)
    at cmp (/opt/boxen/nodenv/versions/v0.10.17/lib/node_modules/gitbook-cli/node_modules/semver/semver.js:490:22)
    at Comparator.test (/opt/boxen/nodenv/versions/v0.10.17/lib/node_modules/gitbook-cli/node_modules/semver/semver.js:560:10)
    at testSet (/opt/boxen/nodenv/versions/v0.10.17/lib/node_modules/gitbook-cli/node_modules/semver/semver.js:894:17)
    at Range.test (/opt/boxen/nodenv/versions/v0.10.17/lib/node_modules/gitbook-cli/node_modules/semver/semver.js:886:9)
    at Function.satisfies (/opt/boxen/nodenv/versions/v0.10.17/lib/node_modules/gitbook-cli/node_modules/semver/semver.js:907:16)
    at checkVersion (/opt/boxen/nodenv/versions/v0.10.17/lib/node_modules/gitbook-cli/lib/versions.js:21:16)
    at /opt/boxen/nodenv/versions/v0.10.17/lib/node_modules/gitbook-cli/lib/versions.js:46:10

~/git/gitbook version/2.0*
❯

@SamyPesse
Copy link
Member Author

@AaronO this is related to gitbook-cli, post an issue there.

@SamyPesse SamyPesse merged commit d72c397 into master Mar 9, 2015
@SamyPesse SamyPesse deleted the version/2.0 branch March 11, 2015 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants