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

action.cjs: escape diff #224

Merged
merged 1 commit into from
Dec 19, 2024
Merged

action.cjs: escape diff #224

merged 1 commit into from
Dec 19, 2024

Conversation

thypon
Copy link
Member

@thypon thypon commented Dec 19, 2024

No description provided.

Copy link

openai debug - [puLL-Merge] - brave/pull-merge@224

Description

This pull request modifies the logic for displaying diff information in a markdown code block. The motivation for this change seems to be to escape backticks within the diff content to ensure proper formatting within the markdown structure.

Changes

Changes

  • action.cjs:
    • Updated the header constant construction. If options.include_diff is true or the context.actor is a bot, the code now replaces any backticks within the filteredPatch with escaped backticks to avoid markup conflicts.

This review should help ensure that the changes improve the markdown rendering of diffs when they include backticks. Everything seems to be covered correctly in this straightforward change.```mermaid
sequenceDiagram
participant User
participant System
participant Github
participant ActionHandler

User->>System: Reviews code
System->>Github: Fetch context and inputs
Github->>System: Returns context and inputs
System->>ActionHandler: Submit review with diff
ActionHandler->>System: Process diff content
Note right of ActionHandler: Escape backticks in diff
System->>User: Show properly formatted diff
</details>

<!-- Generated by gpt-4o-2024-05-13 -->

Copy link

bedrock debug - [puLL-Merge] - brave/pull-merge@224

Description

This PR modifies the action.cjs file in the brave/pull-merge repository. The change is focused on improving the handling of code blocks within the diff summary in pull request reviews. Specifically, it addresses the potential issue of nested code blocks by escaping backticks in the filtered patch.

Changes

Changes

action.cjs:

  • Modified the construction of the header variable when including a diff in the review.
  • Added a replace() function to escape backticks (``) in the filteredPatch.
sequenceDiagram
    participant A as Action
    participant G as GitHub API
    A->>A: Construct header with diff
    A->>A: Escape backticks in filteredPatch
    A->>G: Submit review with modified header
    G-->>A: Confirm review submission
Loading

Copy link

anthropic debug - [puLL-Merge] - brave/pull-merge@224

Description

This PR modifies the action.cjs file to escape backticks (```) within the filteredPatch content when generating the diff header. This change is likely motivated by the need to prevent syntax highlighting issues or potential rendering problems in the GitHub UI when the patch content contains triple backticks.

Changes

Changes

File: action.cjs

  • Modified the generation of the header variable
  • Added a replace() function to escape triple backticks in the filteredPatch content

The change replaces:

'<details><summary>Diff</summary>\n\n```diff\n\n' + filteredPatch + '\n\n```\n\n</details>'

with:

'<details><summary>Diff</summary>\n\n```diff\n\n' + filteredPatch.replace('```', '\\`\\`\\`') + '\n\n```\n\n</details>'

This modification ensures that any triple backticks within the filteredPatch content are escaped, preventing them from interfering with the code block formatting in the generated diff.

sequenceDiagram
    participant A as Action
    participant G as GitHub API
    A->>A: Generate header with diff
    A->>A: Escape triple backticks in filteredPatch
    A->>G: Submit review with escaped diff
    G->>GitHub: Display review with properly formatted diff
Loading

@thypon thypon merged commit fa5bc3f into main Dec 19, 2024
7 checks passed
@thypon thypon deleted the fix/escape-diff branch December 19, 2024 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant