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

Switch from `` code blocks to 4 spaces #758

Closed
wants to merge 1 commit into from

Conversation

jedahan
Copy link
Contributor

@jedahan jedahan commented Feb 4, 2016

This makes it a lot easier to copy paste with the mouse in the commandline.

I am testing now with a few implementations to make sure it works fine.

@igorshubovych
Copy link
Collaborator

Oh, man.
You did so much work without asking ahead. Please tell me that you did some script for it and you did not do it manually.

In cases like this it is better to discuss it in tickets first, before doing any job. This will be most likely rejected, because linter and the most of the clients depends on the backticks when parsing. Usually clients parse the format and present in the nice way.

E.g. NodeJS client present it this way
1 igor igors-macbook-air projects tldr zsh 2016-02-04 21-53-27

It is quite easy to do copy-pasting.

@jedahan
Copy link
Contributor Author

jedahan commented Feb 4, 2016

It was a one line script with sed, so not a lot of work. This was kinda the introduction to discuss the change. I would have to change https://github.com/tldr-pages/tldr-lint/blob/master/lib/tldr-parser.js#L736 and https://github.com/tldr-pages/tldr-lint/blob/master/lib/tldr-parser.js#L758 to allow for 8 spaces, etc.

When using a client, they are formatted in an easy way to copy paste, but if you open the markdown files themselves it is a bit tougher. Of course most people should be using a client, but I thought it looks nicer with 8 spaces (and matches the output of most tools that parse it).

Would people be vehemently against changing the tldr-linter to look for 8 spaces instead of backticks?

I feel like it would actually remove some of the client-side work but maybe i'm totally off there...

@jedahan
Copy link
Contributor Author

jedahan commented Feb 4, 2016

nevermind, saw a bunch of implementations, they rely on '`'

@jedahan jedahan closed this Feb 4, 2016
@waldyrious
Copy link
Member

I would still like this to be discussed. See #686 (comment)

@rubenvereecken
Copy link
Contributor

Poor man actually checked out the generated parser file.

@jedahan
Copy link
Contributor Author

jedahan commented Feb 6, 2016

Well, never worked with any lexer or parser, so didnt even look at the .yy or .l files. Still don't know if these are the right places to change things, but:

https://github.com/tldr-pages/tldr-lint/blob/master/tldr.yy#L53-L55 and https://github.com/tldr-pages/tldr-lint/blob/master/tldr.l#L138-L141 probably relevant.

Somewhat related, maybe generated files shouldn't be checked into the source repo? But I don't see how the generated files are made. Maybe a makefile or just that jison thing?

@rubenvereecken
Copy link
Contributor

I had that problem as well, but I needed the generated files to be present upon npm install. A work around would be to use jison upon tldr-lint startup to generate the needed source code, but that's not a very pretty way to go about it.

Anyway, we'll continue the conversation

@waldyrious waldyrious added the syntax Issues related to the content and markup of the pages. label Feb 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
syntax Issues related to the content and markup of the pages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants