-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
sic: add page #5694
sic: add page #5694
Conversation
The build for this PR failed with the following error(s):
Please fix the error(s) and push again. |
The build for this PR failed with the following error(s):
Please fix the error(s) and push again. |
The build for this PR failed with the following error(s):
Please fix the error(s) and push again. |
suckless doesn't use Capital letters. Can we get a pass on |
The build for this PR failed with the following error(s):
Please fix the error(s) and push again. |
The build for this PR failed with the following error(s):
Please fix the error(s) and push again. |
Do you mean that "sic" is meant to stand for "simple irc client", all in lowercase? I don't think we need to conform to stylistic preferences of the projects we document, as long as the content itself is accurate. We can accommodate stylization of names themselves (as we do e.g. with LaTeX), but the descriptions in tldr pages are meant to be useful and informative, not to represent the project's branding. I would go with:
|
I think it should be fine to just use |
There was a problem hiding this 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, @Waples! I've left some comments below for you to review.
pages/linux/sic.md
Outdated
|
||
- Usage with highlighted alerts on mention of your username: | ||
|
||
`sic | awk '/username/ {printf "\a"}1'` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example seems to demonstrate the usage of the awk
command, rather than the sic
command. Generally in tldr pages we try to keep pages focused on the command they are about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this example demonstrates how you can highlight a message when your username is mentioned when using sic
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm not sure if we should keep this example.
Yes, |
Co-authored-by: Starbeamrainbowlabs <sbrl@starbeamrainbowlabs.com>
The build for this PR failed with the following error(s):
Please fix the error(s) and push again. |
@sbrl you should try out |
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
The build for this PR failed with the following error(s):
Please fix the error(s) and push again. |
Regarding the casing, I admit that I agree with @waldyrious's opinion. As he stated, we're not required to adhere to the projects particular formats, and as 'simple irc client' is merely a description, changing it to match our existing ~3000 pages is the best course of action. Over the years, there have been many such discussions about this exemption or that, but in the end, the simplest, approach most consistent with the rest of the pages is chosen, because otherwise the exceptions pile up and become very difficult to manage. |
@bl-ue then what ya think about.... I've got more |
simple irc client is also the name (hence sic) |
What do you think? |
already changed that. Shall i remove the link from the 2nd description line? |
I would; doesn't seem that useful given it's on the very next line (albeit slightly dissimilar, I don't think it matters much though) |
fixed |
bump @sbrl |
@Waples thanks for your patience with our review comments! The page looks quite good now — I left just a few comments inline, please take a look. |
Co-authored-by: Waldir Pimenta <waldyrious@gmail.com>
Co-authored-by: Waldir Pimenta <waldyrious@gmail.com>
Co-authored-by: Waldir Pimenta <waldyrious@gmail.com>
applied suggestions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM — thanks again @Waples for the submission and patience! Since this already has two approvals, I'll go ahead and merge after I add my own.
The requested changes have been implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for not getting back to this. @Waples: That awk
example was indeed showing highlighted messages, but it did not do so by calling sic
with different arguments - hence it would be a better fit for a wider cheat sheet as opposed to a compact tldr page.
No need to send bump messages to get my attention - it was in my notifications list - I just haven't had time to return to this until now.
common/
,linux/
, etc.)