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

sdcv: add page #6145

Merged
merged 28 commits into from
Jul 7, 2021
Merged

sdcv: add page #6145

merged 28 commits into from
Jul 7, 2021

Conversation

258204
Copy link
Collaborator

@258204 258204 commented Jun 20, 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).

@tldr-bot

This comment has been minimized.

@258204
Copy link
Collaborator Author

258204 commented Jun 20, 2021

The --json option isn't working properly, but -j is, which is why I didn't use --json.

@tldr-bot

This comment has been minimized.

@tldr-bot

This comment has been minimized.

@tldr-bot

This comment has been minimized.

I don't have the slightest Idea how this got here....
@navarroaxel navarroaxel added the new command Issues requesting creation of a new page. label Jun 20, 2021
pages/common/sdcv.md Outdated Show resolved Hide resolved
pages/common/sdcv.md Outdated Show resolved Hide resolved
pages/common/sdcv.md Outdated Show resolved Hide resolved
pages/common/sdcv.md Outdated Show resolved Hide resolved
258204 and others added 3 commits June 21, 2021 01:37
Co-authored-by: Axel Navarro <navarroaxel@gmail.com>
Co-authored-by: Axel Navarro <navarroaxel@gmail.com>
Co-authored-by: CleanMachine1 <78213164+CleanMachine1@users.noreply.github.com>
@tldr-bot

This comment has been minimized.

Co-authored-by: Axel Navarro <navarroaxel@gmail.com>
@tldr-bot

This comment has been minimized.

@258204
Copy link
Collaborator Author

258204 commented Jun 20, 2021

@navarroaxel I checked and all your requested changes were implemented, yet it says there is still one change requested from you? Anything else you want changed?

@258204 258204 requested a review from navarroaxel June 20, 2021 16:11
@tldr-bot

This comment has been minimized.

pages/common/sdcv.md Outdated Show resolved Hide resolved
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
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.

Will you please move the Display help example to the bottom (help usually goes there), and the Start sdcv interactively one to the top (simplest invocations start at the top).

258204 and others added 2 commits June 21, 2021 05:21
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
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! I've left some comments below for you to review.

Comment on lines 3 to 4
> StarDict console version. Command line dictionary client.
> Dictionaries are provided separately.
Copy link
Member

@sbrl sbrl Jun 20, 2021

Choose a reason for hiding this comment

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

Perhaps this will read better?

Suggested change
> StarDict console version. Command line dictionary client.
> Dictionaries are provided separately.
> StarDict command-line dictionary client.
> Dictionaries are provided separately to the program itself.

Copy link
Member

@CleanMachine1 CleanMachine1 Jun 20, 2021

Choose a reason for hiding this comment

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

Would you like to edit that to command-line Edit: done by @bl-ue

I didn't see it previously

And I don't like that itself, its unrequired

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I want to include"StarDict command-line version" somewhere, because that's what the acronym is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about "StarDict command-line version, dictionary client. Dictionaries are provided separately from the client. "

Copy link
Contributor

@bl-ue bl-ue Jun 21, 2021

Choose a reason for hiding this comment

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

Ah, that's interesting @258204. In that case I think it's fine to deviate a bit from "convention."

> StarDict command-line version (sdcv).
> Dictionaries are provided separately from the program itself.

(note: @sbrl's separately to is not grammatically correct — @258204's separately from is)

Copy link
Member

Choose a reason for hiding this comment

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

Hrm, StarDict command-line version doesn't really read very well or make grammatical sense. Despite this, I think @bl-ue's suggestion is the best one given the situation.

Copy link
Member

Choose a reason for hiding this comment

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

What about

> StarDict command-line variant

Copy link
Member

Choose a reason for hiding this comment

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

Hrm. I think StarDict command-line variant is worse than @bl-ue's version.

pages/common/sdcv.md Outdated Show resolved Hide resolved
pages/common/sdcv.md Outdated Show resolved Hide resolved
pages/common/sdcv.md Outdated Show resolved Hide resolved
pages/common/sdcv.md Outdated Show resolved Hide resolved
pages/common/sdcv.md Outdated Show resolved Hide resolved
258204 and others added 5 commits June 21, 2021 09:24
Co-authored-by: Starbeamrainbowlabs <sbrl@starbeamrainbowlabs.com>
Co-authored-by: Starbeamrainbowlabs <sbrl@starbeamrainbowlabs.com>
Co-authored-by: Starbeamrainbowlabs <sbrl@starbeamrainbowlabs.com>
Co-authored-by: Starbeamrainbowlabs <sbrl@starbeamrainbowlabs.com>
@258204
Copy link
Collaborator Author

258204 commented Jun 21, 2021

There were some conflicting changes requested. I modified the file to address them. Let me know if you want any additional changes.

Copy link
Collaborator

@marchersimon marchersimon left a comment

Choose a reason for hiding this comment

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

Just a few suggestions

pages/common/sdcv.md Outdated Show resolved Hide resolved
pages/common/sdcv.md Show resolved Hide resolved
pages/common/sdcv.md Outdated Show resolved Hide resolved
pages/common/sdcv.md Outdated Show resolved Hide resolved
pages/common/sdcv.md Outdated Show resolved Hide resolved
258204 and others added 3 commits July 6, 2021 09:35
Co-authored-by: marchersimon <50295997+marchersimon@users.noreply.github.com>
Co-authored-by: marchersimon <50295997+marchersimon@users.noreply.github.com>
Co-authored-by: marchersimon <50295997+marchersimon@users.noreply.github.com>
@258204 258204 requested review from marchersimon and mfrw July 6, 2021 00:11
pages/common/sdcv.md Show resolved Hide resolved
pages/common/sdcv.md Outdated Show resolved Hide resolved
pages/common/sdcv.md Outdated Show resolved Hide resolved
258204 and others added 2 commits July 8, 2021 03:02
Co-authored-by: marchersimon <50295997+marchersimon@users.noreply.github.com>
Co-authored-by: marchersimon <50295997+marchersimon@users.noreply.github.com>
@258204 258204 requested a review from marchersimon July 7, 2021 17:34
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 sticking with the review process, @258204 :-)

@sbrl sbrl merged commit 6b8fdbe into tldr-pages:main Jul 7, 2021
@258204 258204 deleted the sdcv branch November 28, 2021 02:48
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.

8 participants