Skip to content

Prevent linkify-code from running multiple times #4613

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.

Sign up for GitHub

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

Jump to bottom
Merged
fregante merged 6 commits into from
Jul 28, 2021
Merged

Prevent linkify-code from running multiple times #4613

fregante merged 6 commits into from
Jul 28, 2021

Conversation

@kidonng
Copy link
Member

@kidonng kidonng commented Jul 26, 2021 •
edited
Loading

Fix #3226

Test URLs

From #2066:

URLs in diffs
Homebrew/homebrew-core#18015 (files)

URLs in regular files
/source/features/copy-markdown.js@3f5fc48#L45

URLs in PR files
#546 (files)

URLs in embedded code
https://github.com/sindresorhus/refined-github/blob/3f5fc489e417d4f4a14da5ea423775e9ca9246fd/source/features/copy-markdown.js#L45

Issue reference in file

/source/features/sort-issues-by-update-time.tsx@2ade9a8#L18

Issue reference in embedded code

https://github.com/sindresorhus/refined-github/blob/2ade9a84b1894c7879fab9d3e2d045e876f941a6/source/features/sort-issues-by-update-time.tsx#L18

From #3226:

Screenshot

Before:
image

After:
image

kidonng
kidonng commented Jul 26, 2021
View reviewed changes
@kidonng kidonng changed the title Prevent linkify-codefrom running multiple times Prevent linkify-code from running multiple times Jul 27, 2021
@kidonng
Copy link
Member Author

kidonng commented Jul 27, 2021

Explaining the selector changes:

  • .comment-body is dropped because all code inside should also be included in .blob-wrapper

    The .d-block part is from a previous fix but we don't need it anymore

  • .blob-expanded is dropped because it is covered by .blob-wrapper tr

  • We are now using more specific selectors to select tr (.js-blob-wrapper tr and .blob-wrapper tr), because code blocks inside comments are also inside file blocks, but linkifiedURLClass will prevent the linkifying from running twice.

  • We still need the :not(.inline-comments) part for .js-blob-wrapper tr because code blocks inside .inline-comments is already handled by .blob-wrapper tr.

@fregante
Copy link
Member

fregante commented Jul 27, 2021 •
edited
Loading

Thanks! Could you add a few comments explaining where each selector runs, like

// isSingleFile, isPRFiles review suggestion, etc

@fregante fregante added the bug label Jul 27, 2021
@kidonng
Copy link
Member Author

kidonng commented Jul 27, 2021

I don't think it make much sense to list every page there so keep in mind that .js-blob-wrapper is for file blocks, .blob-wrapper for anything else and that's it.

@fregante fregante merged commit 4a60549 into refined-github:main Jul 28, 2021
@fregante
Copy link
Member

fregante commented Jul 28, 2021

Thanks!

image

@kidonng kidonng deleted the linkify-code branch August 2, 2021 18:47
@kidonng kidonng mentioned this pull request Aug 20, 2021
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fregante fregante fregante left review comments

Assignees

No one assigned

Labels

bug

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

linkify-code runs multiple times on the same URL

2 participants

@kidonng @fregante