rethinkdb / docs

RethinkDB documentation
http://rethinkdb.com/docs
Apache License 2.0
117 stars 167 forks source link

Standardise JavaScript code examples #1195

Closed rushmorem closed 7 years ago

rushmorem commented 7 years ago

This commit standardises the JavaScript code examples to make them easier to parse using automated tools.

Description

Using javascript instead of js makes it easier for parsers to differentiate the code blocks from json code blocks. This pull request also makes the text code blocks explicit. I'm generating a Rust driver from the JavaScript documentation, so I needed to make these changes.

AtnNn commented 7 years ago

Is there a difference in highlighting between js and javascript?

Why are these changes necessary? How is js hard to tell apart from json?

rushmorem commented 7 years ago

Is there a difference in highlighting between js and javascript?

There is no difference. Currently almost all the examples are using js except for one example which is using javascript. That is the ungroup "finding the arithmetic mode of an array of values" example.

Why are these changes necessary? How is js hard to tell apart from json?

It's not hard. It's just that javascript is easier to write a parser for than js if the same body of text also contains json. This is because both js and json start with "js".

Another reason I didn't mention in the commit message is that javascript is more consistent with how the driver is referred to everywhere else.

AtnNn commented 7 years ago

I would accept a patch that changed the lone ```javascript into ```js.

We use rb and py in the rest of the docs.

How hard can it be to tell the difference between js and json?

AtnNn commented 7 years ago

I didn't mean to be rude, I'm just surprised. I would love to help with any change that would make it easier to develop and maintain the rust driver.

rushmorem commented 7 years ago

I would accept a patch that changed the lone javascript` intojs`.

That's not a problem. I could simply change the javascript to js and push again. I don't think closing this pull request without consulting me first was necessary. The only reason I used javascript, like I said was because it's easier.

We use rb and py in the rest of the docs.

Right, but these are also just as easy to parse against as javascript because none of them start with "js".

This pull request also contained a couple of other fixes.

I didn't mean to be rude, I'm just surprised. I would love to help with any change that would make it easier to develop and maintain the rust driver.

With all due respect, I do understand that you found this surprising but I did this out of necessity because it makes things easier for me. For example check out this parser I have right now. If we use js instead of javascript I will then have to write additional logic because js would also match json when I only want to grab the JavaScript code blocks.

What I don't understand though, surprised by this pull request or not, is why you chose to just close this pull request. It doesn't make any difference on the API docs whether we are using javascript or js.

AtnNn commented 7 years ago

If we use js instead of javascript I will then have to write additional logic

Is that the nom parsing library? I've never used it, but I'm sure it can check for newlines. The documentation even mentions the eol and line_ending library functions that I believe you could use.

What I don't understand though [...] is why you chose to just close this pull request

I didn't just close it, I wrote a comment explaining why. We always close pull requests we're not going to merge, otherwise it clutters the list of open pull requests.

This pull request also contained a couple of other fixes.

Sorry, I only saw one commit and didn't notice any other changes in it. I'll re-open.

rushmorem commented 7 years ago

I have updated the pull request to use js instead of javascript.

Edit: The changes aren't reflecting here since this pull request has already been closed. You can see all the changes in this commit. It's just 5 files now.

rushmorem commented 7 years ago

Is that the nom parsing library? I've never used it, but I'm sure it can check for newlines. The documentation even mentions the eol and line_ending library functions that I believe you could use.

Yes it is and yes it does. That's the additional logic I was referring to. I never said it was hard, only that using javascript was easier. Anyway, I have decided to handle both cases to make the parser more robust against future changes.

I didn't just close it, I wrote a comment explaining why.

By "just" closing, I was referring to the fact that you just closed the pull request without first asking if I was willing to update it. Anyway I'm very sorry if I overreacted to your responses. I love RethinkDB and I really appreciate all the work you guys have put into it. Thank you for that.

rushmorem commented 7 years ago

@AtnNn Are you still going to reopen this or should I submit another pull request?

AtnNn commented 7 years ago

I thought I had re-opened it. Github is now refusing to re-open, it says you've deleted the branch.

rushmorem commented 7 years ago

I didn't delete, the branch is still there. Perhaps it's because I rebased the previous commit. I will open another pull request. Thanks.