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

fmt: add page and Italian translation #2764

Merged
merged 2 commits into from
Feb 9, 2019

Conversation

manabiseijin
Copy link
Contributor

fmt pages in English and Italian for #2213.

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

@manabiseijin manabiseijin reopened this Feb 8, 2019
@tldr-bot
Copy link

tldr-bot commented Feb 8, 2019

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

pages/common/fmt.md:20: TLDR008 File should contain no trailing whitespace

Please fix the error(s) and push again.

@tldr-bot
Copy link

tldr-bot commented Feb 8, 2019

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

pages/common/fmt.md:20: TLDR008 File should contain no trailing whitespace

Please fix the error(s) and push again.

@owenvoke owenvoke added new command Issues requesting creation of a new page. translation Translate pages from one language to another. labels Feb 8, 2019
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, @gRastello! I've left some comments below for you to review.

pages/common/fmt.md Outdated Show resolved Hide resolved
pages/common/fmt.md Outdated Show resolved Hide resolved
@manabiseijin
Copy link
Contributor Author

Suggestions seems good to me, applied to both English and Italian pages.

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, @gRastello :smiley

Copy link
Member

@mebeim mebeim left a comment

Choose a reason for hiding this comment

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

Hello again, @gRastello and thank you.

I see you are making a PR from a branch which has 16 commits, some of which were already incorporated in a previous PR and have nothing to do with the requested page addition. This makes it hard to understand which changes are going to be applied. Could you please rebase your branch on upstream/master to only keep the relevant commits?

pages/common/fmt.md Outdated Show resolved Hide resolved
pages.it/common/fmt.md Outdated Show resolved Hide resolved
pages/common/fmt.md Outdated Show resolved Hide resolved
pages.it/common/fmt.md Outdated Show resolved Hide resolved
@manabiseijin
Copy link
Contributor Author

I do agree on using "paragraph" instead of "sentence" since it makes more intuitive sense. However the official documentation as well as the man page do use the word "sentence" defining a sentence break as the end of a paragraph. Pretty confusing to be fair.

Also sorry for the mess I made with the branch but it should be fixed now.

pages/common/fmt.md Outdated Show resolved Hide resolved
@mebeim
Copy link
Member

mebeim commented Feb 9, 2019

@gRastello the documentation talks about sentences because it says that breaking lines is done preferring the end of sentences by default. That specific command I was commenting on adds two spaces between paragraphs though.

For example:

$ fmt -u <<< 'abc def. ghijkl.
> abcd. efg.
> hij.'
abc def. ghijkl.  abcd. efg.  hij.

You can see that two spaces are only used between paragraphs, not sentences (paragraph = group of sentences followed by a newline character).

Copy link
Member

@mebeim mebeim left a comment

Choose a reason for hiding this comment

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

Well, looks good to me 👍 thanks @gRastello 😄

@mebeim mebeim merged commit 5bdea20 into tldr-pages:master Feb 9, 2019
@manabiseijin manabiseijin deleted the grastello branch February 10, 2019 08:19
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. translation Translate pages from one language to another.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants