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

Add docs for MaybeSentinel and RemovalSentinel #15

Merged
merged 1 commit into from
Aug 8, 2019

Conversation

bgw
Copy link
Contributor

@bgw bgw commented Aug 8, 2019

Summary

These were referred to by other parts of the documentation, and provide
core parts of our functionality, so we should have explicit
documentation for them.

Test Plan

https://1371-200896124-gh.circle-artifacts.com/0/doc/nodes.html#maybe-sentinel
https://1371-200896124-gh.circle-artifacts.com/0/doc/visitors.html#libcst.RemovalSentinel

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 8, 2019
These were referred to by other parts of the documentation, and they
provide core parts of our functionality, so we should have explicit
documentation for them.
>>> new_fn_call = fn_call.with_changes(
... args=[*fn_call.args, cst.Arg(cst.Integer("3"))]
... )
>>> dummy_module = cst.parse_module("") # we need to use Module.code_for_node
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we provide some helper to render code of a Node (not Module) easily?
What's the reason we don't provide the code property in Node or Statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can only generate code in the context of a module because the module contains information about the default indentation level, newline, etc.

However (as this example demonstrates) it'd be useful to have a .code property on every node for debugging purposes. I just don't know how we can communicate these pitfalls to the developer.

It's also a little weird that parse_expression and parse_statement are lossy and not entirely reversible, but that's by design. If you have a module using \r\n as it's newline format and you use parse_statement to insert a line of code with \n in it, I'd want that \n to be replaced with \r\n (which is the current behavior).

@bgw bgw merged commit 90e4efb into Instagram:master Aug 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants