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

Update docs site for version 2.23.0.dev4 #234

Closed
wants to merge 1 commit into from

Conversation

WorkerPants
Copy link
Member

@WorkerPants WorkerPants added the automation:sync-docs An automatic sync of documentation from the pantsbuild/pants repo label Jul 25, 2024
@WorkerPants WorkerPants requested a review from huonw July 25, 2024 04:41
@@ -61,12 +61,21 @@ If you instead want those files included in any packages specified in the `packa

<Field
type_repr={`str | None`}
default_repr={`None`}
default_repr={`'${spec_path_normalized}/${target_name_normalized}${file_suffix}'`}
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect this is causing the build failures, we'll need to do more escaping.

Copy link
Contributor

Choose a reason for hiding this comment

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

With #235, this now looks like:

@@ -61,7 +61,7 @@ If you instead want those files included in any packages specified in the `packa
 
 <Field
     type_repr={`str | None`}
-    default_repr={`'${spec_path_normalized}/${target_name_normalized}${file_suffix}'`}
+    default_repr={`'\${spec_path_normalized}/\${target_name_normalized}\${file_suffix}'`}
 >
 
 Where the built asset should be located.

And renders "properly":

image

@huonw
Copy link
Contributor

huonw commented Jul 25, 2024

Closing. I'll recreate once #235 is in.

@huonw huonw closed this Jul 25, 2024
@huonw huonw deleted the automation/sync-2.23.0.dev4 branch July 25, 2024 06:26
huonw added a commit that referenced this pull request Jul 25, 2024
This escapes `$` in default values to avoid them being treated as
template literal substitutions.

Default values end up in a template literal `` `something` ``. If the
string contains `${...}` literally, they'll be treated as a
substitution. This PR escapes all `$`s to avoid this.

This bug/oversight doesn't cause problems with any _current_ values, but
will be required in 2.23.0.dev4 due to the new default value for
`output_path` fields from
pantsbuild/pants#21175.

Example of the effect (from
#234 (comment)):

```diff
- default_repr={`'${spec_path_normalized}/${target_name_normalized}${file_suffix}'`}
+ default_repr={`'\${spec_path_normalized}/\${target_name_normalized}\${file_suffix}'`}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation:sync-docs An automatic sync of documentation from the pantsbuild/pants repo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants