Skip to content

bpo-33702: Add some missing links in production lists and do a little polish#7259

Merged
serhiy-storchaka merged 7 commits intopython:masterfrom
andresdelfino:add-missing-links
Jul 7, 2018
Merged

bpo-33702: Add some missing links in production lists and do a little polish#7259
serhiy-storchaka merged 7 commits intopython:masterfrom
andresdelfino:add-missing-links

Conversation

@andresdelfino
Copy link
Copy Markdown
Contributor

@andresdelfino andresdelfino commented May 30, 2018 •
edited by bedevere-bot
Loading

@serhiy-storchaka serhiy-storchaka self-requested a review May 31, 2018 04:32
@andresdelfino andresdelfino changed the title bpo-33702: Add some missings links in production lists and a little polish bpo-33702: Add some missing links in production lists and do a little polish Jun 11, 2018
serhiy-storchaka
serhiy-storchaka reviewed Jul 7, 2018
View reviewed changes
Doc/reference/compound_stmts.rst Outdated
parameter_list_starargs: "*" [`parameter`] ("," `defparameter`)* ["," ["**" `parameter` [","]]]
: | "**" `parameter` [","]
parameter_list: `defparameter` ("," `defparameter`)*
: ["," [`parameter_list_starargs`]] | `parameter_list_starargs`
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Jul 7, 2018

Choose a reason for hiding this comment

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

I think it is better to left this and the following production lists as is.

Doc/reference/expressions.rst Outdated
slice_list: `slice_item` ("," `slice_item`)* [","]
slice_item: `expression` | `proper_slice`
proper_slice: [`lower_bound`] ":" [`upper_bound`] [ ":" [`stride`] ]
proper_slice: [`lower_bound`] ":" [`upper_bound`] [":" [`stride`]]
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Jul 7, 2018

Choose a reason for hiding this comment

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

I think a space between ]'s increase readability. And the space after [ was added for symmetry.

Doc/reference/expressions.rst Outdated
call: `primary` "(" [`argument_list` [","] | `comprehension`] ")"
argument_list: `positional_arguments` ["," `starred_and_keywords`]
: ["," `keywords_arguments`]
: ["," `keywords_arguments`]
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Jul 7, 2018

Choose a reason for hiding this comment

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

Left it as is. Extra spaces help to see a difference between the continuation and the alternation.

Doc/reference/lexical_analysis.rst Outdated
replacement_field: "{" `f_expression` ["!" `conversion`] [":" `format_spec`] "}"
f_expression: (`conditional_expression` | "*" `or_expr`)
: ("," `conditional_expression` | "," "*" `or_expr`)* [","]
: ("," `conditional_expression` | "," "*" `or_expr`)* [","]
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Jul 7, 2018

Choose a reason for hiding this comment

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

Same as above.

Doc/reference/simple_stmts.rst
: ( "," `identifier` ["as" `name`] )*
: | "from" `relative_module` "import" "(" `identifier` ["as" `name`]
: ( "," `identifier` ["as" `name`] )* [","] ")"
import_stmt: "import" `module` ["as" `identifier`] ("," `module` ["as" `identifier`])*
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Jul 7, 2018

Choose a reason for hiding this comment

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

Why name was replaced by identifier?

Copy link
Copy Markdown
Contributor Author

@andresdelfino andresdelfino Jul 7, 2018

Choose a reason for hiding this comment

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

Because, when I added a link to name in future statements, it linked to the name in import statements. That's why I replaced name in both places to "identifier". Perhaps it can be solved in other way?

Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Jul 7, 2018

Choose a reason for hiding this comment

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

Ah, I see, the problem was that name was defined in two places. Then using an identifier LGTM. In the Grammar file the same term is used for the names before and after 'as' in import_from.

serhiy-storchaka
serhiy-storchaka reviewed Jul 7, 2018
View reviewed changes
Doc/reference/compound_stmts.rst Outdated
: | `parameter_list_starargs`
parameter_list_starargs: "*" [`parameter`] ("," `defparameter`)* ["," ["**" `parameter` [","]]]
: | "**" `parameter` [","]
parameter_list_starargs: "*" [`parameter`] ("," `defparameter`)*
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Jul 7, 2018

Choose a reason for hiding this comment

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

It is better to break a line before | than at arbitrary place. I think it is better to keep the existing formatting. It is not much longer that the previous production list.

Doc/reference/simple_stmts.rst
: ( "," `identifier` ["as" `name`] )*
: | "from" `relative_module` "import" "(" `identifier` ["as" `name`]
: ( "," `identifier` ["as" `name`] )* [","] ")"
import_stmt: "import" `module` ["as" `identifier`] ("," `module` ["as" `identifier`])*
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Jul 7, 2018

Choose a reason for hiding this comment

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

Ah, I see, the problem was that name was defined in two places. Then using an identifier LGTM. In the Grammar file the same term is used for the names before and after 'as' in import_from.

@serhiy-storchaka serhiy-storchaka merged commit caccca7 into python:master Jul 7, 2018
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Jul 7, 2018

Thanks @andresdelfino for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 7, 2018
@andresdelfino @miss-islington
bpo-33702: Add some missing links in production lists and do a little…
a3e14f0 
… polish (pythonGH-7259)

(cherry picked from commit caccca7)

Co-authored-by: Andrés Delfino <adelfino@gmail.com>
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Jul 7, 2018

GH-8169 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 7, 2018
@andresdelfino @miss-islington
bpo-33702: Add some missing links in production lists and do a little…
b35150a 
… polish (pythonGH-7259)

(cherry picked from commit caccca7)

Co-authored-by: Andrés Delfino <adelfino@gmail.com>
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Jul 7, 2018

GH-8170 is a backport of this pull request to the 3.6 branch.

@andresdelfino andresdelfino deleted the add-missing-links branch July 7, 2018 21:06
miss-islington added a commit that referenced this pull request Jul 7, 2018
@miss-islington @andresdelfino
bpo-33702: Add some missing links in production lists and do a little…
80c188f 
… polish (GH-7259)

(cherry picked from commit caccca7)

Co-authored-by: Andrés Delfino <adelfino@gmail.com>
miss-islington added a commit that referenced this pull request Jul 7, 2018
@miss-islington @andresdelfino
bpo-33702: Add some missing links in production lists and do a little…
445f9ed 
… polish (GH-7259)

(cherry picked from commit caccca7)

Co-authored-by: Andrés Delfino <adelfino@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@andresdelfino @miss-islington @bedevere-bot @serhiy-storchaka @the-knights-who-say-ni