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

Multiple pages: Add homepages #2684

Merged
merged 7 commits into from
Jan 30, 2019
Merged

Multiple pages: Add homepages #2684

merged 7 commits into from
Jan 30, 2019

Conversation

principis
Copy link
Contributor

Added some more homepages, see: #2660
It seemed handy for all the git pages to link directly to the docs although it's not really a homepage. But it is the same website so... Let me know if I need to change anything.

@tldr-bot
Copy link

tldr-bot commented Jan 7, 2019

The build for this PR has failed with the following error(s):

Error: Parse error on line 32:
...arget_file}}.patch`> Homepage: <https:/
----------------------^
Expecting 'NEWLINE', 'TEXT', 'DASH', 'BACKTICK', got 'GREATER_THAN'

Please fix the error(s) and push again.

@blunket
Copy link
Contributor

blunket commented Jan 7, 2019

(Sorry for my previous comment -- I screwed up, lol)

@principis
Copy link
Contributor Author

(Sorry for my previous comment -- I screwed up, lol)

NP :) my script messed it up.

Copy link
Member

@sbrl sbrl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @principis 😺

@sbrl sbrl added page edit Changes to an existing page(s). mass changes Changes that affect multiple pages. labels Jan 7, 2019
@waldyrious
Copy link
Member

@sbrl what happened to your insistence on using the slashes at the end of URLs? ;)

In all seriousness, though, I think it might be better if we don't include them because people are more likely to write links without a trailing slash than with them. If we insist in keeping them, we'd have to constantly remind people of them -- case in point -- which is a bit of a drag for everyone.

Btw, @principis, I agree that using specific pages for each subcommand is more useful than linking all of them to the homepage of the main command. Perhaps we should use a different term than "homepage" for the main link, to avoid giving out the wrong idea. WDYT?

@principis
Copy link
Contributor Author

principis commented Jan 8, 2019

@waldyrious I added slashes consistently with the websites as was said in #2660 I find them ugly so I would rather remove them but it's not that big of a deal.

Perhaps we should use a different term than "homepage" for the main link, to avoid giving out the wrong idea. WDYT?

Ow I forgot to mention that sorry 😆 It was my intention to mention it in my PR.
I couldn't think of a good term but I think it's a good idea.

Maybe something that means "more info"

@jedahan
Copy link
Contributor

jedahan commented Jan 8, 2019

maybe we don't specify and let people put whatever they think is most appropriate? Most likely it will be
homepage: http://mycoolproject.org
but sometimes
documentation: http://project.readthedocs.org
and maybe even
community: http://project.gitter.im

@sbrl
Copy link
Member

sbrl commented Jan 8, 2019

@waldyrious: Oops! Missed that :P I'm unsure that it's worth ensuring that all urls are formatted in exactly the same way though.

@jedahan: Sounds like a great idea!

@waldyrious
Copy link
Member

I still vote for consistently not adding slashes :) they're redundant and quite prone to cause inconsistencies, as we've seen here and even in #2660.

As for making the URL free-form, I am not convinced it would be a good idea. I'm inclined to agree with @principis' suggestion of "More info:". Or we could simply call it "Link:" or "Web page:". Any of these could be used for all URLs, without the need to add different labels for different pages.

@sbrl
Copy link
Member

sbrl commented Jan 12, 2019

Sounds good to me, @waldyrious

@@ -1,6 +1,7 @@
# youtube-dl

> Download videos from YouTube and other websites.
> Homepage: <http://rg3.github.io/youtube-dl/>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer https if it is available :bowtie:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbrl Is this linter (https://github.com/tldr-pages/tldr-lint) still used for validating tldr pages? If so this should be added as rule there...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to comment on this PR ? What has tldr-lint got to do with adding a homepage link ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ideal world tldr-lint or any other linter used for checking tldr pages should report if http (home)page is linked and https version is available. It should report dead links also?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see. I think that would put too much logic into the linter. I would want the heading text to be free-form without any strict format. The Homepage: format is just something we agreed to. I think it is not necessary to have linter checks for that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some commands' homepages may be only available over http and not https though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quoting myself:

if http (home)page is linked and https version is available.

That is relatively easy to check.

Copy link
Member

@sbrl sbrl Jan 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

....in C/C++ with bison?

We need to do something about the maintainability of that linter at some point. I've studied bison at University since the last time I looked at it, and I think it might be overkill here.

I think a simple lexer + recursive descent parser would be more maintainable. IIRC, we discussed it elsewhere, but I can't remember where.

Copy link
Contributor

@vladimyr vladimyr Jan 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably not a place to discuss it but parsing tldr pages with bison is like killing fly with orbit ion cannon 🙄
After all tldr page is markdown document and there are plenty of markdown parsers around that can be adapted/wrapped to check page format. Such parser could provide hooks for specific page segments like urls which can in turn be used for http/https and/or dead link checking.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't agree more. Sadly, that is the state of affairs now. If someone wants to rewrite the linter completely, they are more than welcome.

@sbrl
Copy link
Member

sbrl commented Jan 28, 2019

What's remaining for getting this and #2660 merged, @agnivade?

@agnivade
Copy link
Member

We were waiting for the node client to have home pages support. Currently, the node client breaks trying to display the home page link.

@vladimyr - You had mentioned about subclassing the markdown parser to add the support. Are you planning to do that ?

@vladimyr
Copy link
Contributor

Not atm 😞 Right now I don't have enough free time for doing it properly including extensive test suite but maybe some time in the future...

@agnivade
Copy link
Member

Ok, how about we do @jedahan's solution so that this is unblocked for now, and you can do it properly later whenever you have time ?

@vladimyr
Copy link
Contributor

Sure, totally agree 👍

@agnivade
Copy link
Member

Done. Merging this.

@agnivade agnivade merged commit 672d93a into tldr-pages:add-homepages Jan 30, 2019
@waldyrious waldyrious mentioned this pull request Feb 28, 2019
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mass changes Changes that affect multiple pages. page edit Changes to an existing page(s).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants