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

bpo-29928: Add f-string to the Glossary #864

Merged
merged 6 commits into from
Mar 30, 2017
Merged

Conversation

Mariatta
Copy link
Member

No description provided.

@Mariatta Mariatta added docs Documentation in the Doc dir needs backport to 3.6 labels Mar 28, 2017
@mention-bot
Copy link

@Mariatta, thanks for your PR! By analyzing the history of the files in this pull request, we identified @birkenfeld, @benjaminp and @tiran to be potential reviewers.

Doc/glossary.rst Outdated
@@ -320,6 +320,21 @@ Glossary
A module written in C or C++, using Python's C API to interact with the
core and with user code.

f-string
Short for :dfn:`formatted string literal`, which is string literal
Copy link
Member

Choose a reason for hiding this comment

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

which is [a] string literal

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed :)

Doc/glossary.rst Outdated
@@ -320,6 +320,21 @@ Glossary
A module written in C or C++, using Python's C API to interact with the
core and with user code.

f-string
Short for :dfn:`formatted string literal`, which is string literal
that is prefixed with ``'f'`` or ``'F'``. f-strings contain
Copy link
Member

Choose a reason for hiding this comment

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

Start sentence with a capital letter: F-strings contain . . .

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm.. I followed PEP 498 where it is not capitalized. https://github.com/python/peps/blob/master/pep-0498.txt#L29

Should the PEP be updated?

Doc/glossary.rst Outdated
"He said his name is 'Fred'."

See also :pep:`498`, the proposal that added formatted string literals,
and the :ref:`documentation <f-strings>`.
Copy link
Contributor

Choose a reason for hiding this comment

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

While this looks good to me would it maybe be better to switch the links around and send people to the docs first and the pep later? I.e something like:

"For more information, see the :ref:documentation on f-strings <f-strings> and :pep:498, the proposal that added them."

What do you think?

Copy link
Contributor

@rhettinger rhettinger left a comment

Choose a reason for hiding this comment

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

Please make this entry as brief as possible. The glossary is a quick term reference, tutorial, or primary source. The best of the glossary entires are short and sweet. Your original wording was closer to the mark. Try something like: String literals prefixed with f or F are commonly called "f-strings" which is short for :dfn:formatted string literal. See also :ref:f-strings <f-strings> and :pep:498.

@Mariatta
Copy link
Member Author

Thanks @rhettinger :) I updated this based on your suggestion.

Doc/glossary.rst Outdated
@@ -320,6 +320,11 @@ Glossary
A module written in C or C++, using Python's C API to interact with the
core and with user code.

f-string
String literals prefixed with f or F are commonly called "f-strings"
which is short for :dfn:`formatted string literal`. See also
Copy link
Member

Choose a reason for hiding this comment

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

This isn't the definition of "formatted string literal" (:dfn: is used for formatting term definitions). The reference "See also f-strings" looks self-recursive.

I think it would be better to write which is short for :ref:'formatted string literal <f-strings>'.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, @serhiy-storchaka :) Updated.

@Mariatta
Copy link
Member Author

Thanks all :)

@Mariatta Mariatta merged commit 33db068 into python:master Mar 30, 2017
@Mariatta Mariatta deleted the bpo-29928 branch March 30, 2017 19:12
Mariatta added a commit to Mariatta/cpython that referenced this pull request Mar 30, 2017
Mariatta added a commit that referenced this pull request Mar 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants