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

test, [: add page, improve examples #5205

Merged
merged 4 commits into from
Mar 10, 2021
Merged

Conversation

Rudloff
Copy link
Collaborator

@Rudloff Rudloff commented Jan 30, 2021

  • The page (if new), does not already exist in the repo.
  • The page is in the correct platform directory (common/, linux/, etc.)
  • The page has 8 or fewer examples.
  • The PR title conforms to the recommended templates.
  • The page follows the content guidelines.
  • The page description includes a link to documentation or a homepage (if applicable).

The examples are reused from test.md.

tldr-lint would not let me use [ as the page title.

@tldr-bot

This comment has been minimized.

@bl-ue bl-ue added the new command Issues requesting creation of a new page. label Jan 30, 2021
@bl-ue
Copy link
Contributor

bl-ue commented Jan 30, 2021

Hey @Rudloff! Wow, this is crazy one...sort of duplicate of another page, also it's name is strange...we should update the linter to allow [ as a page title I guess...

In general, I don't really know what we should do about it, so I'll ask the others who are much more familiar than I am.

sbrl
sbrl previously requested changes Jan 30, 2021
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 for the new page, @Rudloff! I've left some comments below for you to review.

@bl-ue: This is a perfectly legit page :P

This isn't even a shell builtin like shopt (which we also have documented) - try executing which [.

If the linter isn't configured to like this, it should be. It's this kinda command that means linting command names or filenames is rather a challenge :P

pages/common/[.md Outdated Show resolved Hide resolved
pages/common/[.md Outdated Show resolved Hide resolved
pages/common/[.md Outdated Show resolved Hide resolved
pages/common/[.md Outdated Show resolved Hide resolved
@bl-ue
Copy link
Contributor

bl-ue commented Jan 30, 2021

Cool @sbrl. These changes should also be ported to https://github.com/tldr-pages/tldr/blob/master/pages/common/test.md. That page is actually so old (1/8/2016!) and it should be updated in general.
I'll review this PR, too.

Copy link
Contributor

@bl-ue bl-ue left a comment

Choose a reason for hiding this comment

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

Sorry @Rudloff now every description and every command has a suggestion 😂

Well, the page this was based on, test.md, is actually really old. It was added in 2015 and modified only once since.

pages/common/[.md Outdated Show resolved Hide resolved
pages/common/[.md Outdated Show resolved Hide resolved
pages/common/[.md Outdated Show resolved Hide resolved
pages/common/[.md Outdated Show resolved Hide resolved
pages/common/[.md Outdated Show resolved Hide resolved
pages/common/[.md Outdated Show resolved Hide resolved
Rudloff and others added 2 commits January 30, 2021 18:04
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
Co-authored-by: Starbeamrainbowlabs <sbrl@starbeamrainbowlabs.com>
@bl-ue bl-ue requested a review from sbrl January 30, 2021 17:08
@bl-ue
Copy link
Contributor

bl-ue commented Jan 30, 2021

We shouldn't merge this though. The title should be changed to [ and we should wait until the linter supports that...

pages/common/[.md Outdated Show resolved Hide resolved
Co-authored-by: Starbeamrainbowlabs <sbrl@starbeamrainbowlabs.com>
@tldr-bot
Copy link

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

pages/common/[.md:1: TLDR013 Title should be alphanumeric with dashes, underscores or spaces

Please fix the error(s) and push again.

@bl-ue bl-ue changed the title [: add page test, [: add page, improve examples Jan 31, 2021
@sbrl
Copy link
Member

sbrl commented Feb 12, 2021

Was the linter updated? I can't remember. There's also the lowercase filenames change that was merged into the linter recently too, so caution is probably advised.

@bl-ue
Copy link
Contributor

bl-ue commented Feb 12, 2021

@sbrl your PR tldr-pages/tldr-lint#44 is waiting for you 😉

@sbrl
Copy link
Member

sbrl commented Feb 12, 2021

Thanks, @bl-ue! I've updated there & run jison. I've been busy with life stuff, so haven't had much time for tldr-pages recently :-/

@github-actions
Copy link

Hi all! This thread has not had any recent activity.
Are there any updates? Thanks!

@github-actions github-actions bot added the waiting Issues/PRs with Pending response by the author. label Feb 28, 2021
@bl-ue
Copy link
Contributor

bl-ue commented Feb 28, 2021

Sigh. This could really use #5257. /cc @sbrl @owenvoke

@bl-ue bl-ue removed the waiting Issues/PRs with Pending response by the author. label Feb 28, 2021
@owenvoke
Copy link
Member

I'll release a new version of tldr-lint tomorrow if that's ok? Apologies for the delay, I've had other things to do. 👍

@bl-ue
Copy link
Contributor

bl-ue commented Feb 28, 2021

@owenvoke That would be great! Don't worry about the delay. You're not the only one ;)

@bl-ue bl-ue mentioned this pull request Mar 7, 2021
6 tasks
@bl-ue bl-ue closed this Mar 10, 2021
@bl-ue bl-ue reopened this Mar 10, 2021
@bl-ue
Copy link
Contributor

bl-ue commented Mar 10, 2021

How do we get the CI to re-run? Nevermind, checks are green as soon as I send! :)

Copy link
Contributor

@bl-ue bl-ue left a comment

Choose a reason for hiding this comment

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

Thank you @Rudloff!

@superatomic
Copy link
Contributor

This PR broke fish shell autocompletion in tldr-pages/tldr-c-client.

Fix: tldr-pages/tldr-c-client#86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new command Issues requesting creation of a new page.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants