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

fix(changelog): remove urls from md heading url #23112

Merged

Conversation

setchy
Copy link
Collaborator

@setchy setchy commented Jul 3, 2023

Changes

Removes any urls when generating the markdown header url

Context

Current behavior generates markdown header urls for standard-version changelog files that include the github compare url

This PR ensures that this is omitted when generating the markdown header url link

Test Repository:

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@setchy setchy requested a review from viceice July 3, 2023 15:43
@setchy setchy changed the title feat(changelog): remove urls from md heading url fix(changelog): remove urls from md heading url Jul 3, 2023
@setchy setchy added type:bug Bug fix of existing functionality core:changelogs Related to changelogs/release notes fetching labels Jul 3, 2023
@setchy setchy requested a review from viceice July 3, 2023 16:12
@viceice viceice requested a review from rarkins July 3, 2023 16:37
@rarkins rarkins added this pull request to the merge queue Jul 3, 2023
Merged via the queue into renovatebot:main with commit 8b819dd Jul 3, 2023
38 checks passed
@setchy setchy deleted the feature/remove-urls-from-md-section-url branch July 3, 2023 19:55
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 35.159.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@viceice
Copy link
Member

viceice commented Jul 4, 2023

looks like this broke changelogs?
image

### [`v16.0.2`](https://github.com/primefaces/primeng/blob/HEAD/CHANGELOG.md#1602-2023-06-21)

[Compare Source](https://github.com/primefaces/primeng/compare/16.0.1...16.0.2)

[Full Changelog](https://github.com/primefaces/primeng/compare/16.0.1...16.0.2)

**Fixed bugs:**

-   FileUploader: Choose button is missing icon spacing #​13232](https://github.com/primefaces/primeng/issues#​13232)
-   Compiler error: p-autoComplete Argument of type 'Event' is not assignable to parameter of type 'MouseEvent' #​13227](https://github.com/primefaces/primeng/issues#​13227)
-   p-dropdown unable to auto-select first item if group used #​12637](https://github.com/primefaces/primeng/issues#​12637)
-   Component: Tooltip has no default zindex 16.0.1 #​13220](https://github.com/primefaces/primeng/issues#​13220)
-   Clear filter Icon is not showing in p-columnfilter Component #​12947](https://github.com/primefaces/primeng/issues#​12947)
-   Table Filter: Remove filter button is invisible #​13134](https://github.com/primefaces/primeng/issues#​13134)
-   In table filter slash icon is not displaying when data is entered in the row filter. #​13222](https://github.com/primefaces/primeng/issues#​13222)
-   Error: Cannot resolve type entity SafeHtmlPipe to symbol #​13218](https://github.com/primefaces/primeng/issues#​13218)

@@ -6,7 +6,7 @@ import { regEx } from './regex';
export function sanitizeMarkdown(markdown: string): string {
let res = markdown;
// Put a zero width space after every # followed by a digit
res = res.replace(regEx(/#(\d)/gi), '#​$1');
res = res.replace(regEx(/\W#(\d)/gi), '#​$1');
Copy link
Member

@viceice viceice Jul 4, 2023

Choose a reason for hiding this comment

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

this removes the char before the # 🤦‍♂️

Copy link
Collaborator Author

@setchy setchy Jul 4, 2023

Choose a reason for hiding this comment

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

Do you have an example.... I can add this to the test-cases...

Copy link
Collaborator Author

@setchy setchy Jul 4, 2023

Choose a reason for hiding this comment

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

I think this might be better

regex: (\W)#(\d)
replacement: $1#​$2

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The other route I explored was using %23 instead of the # in release-notes.ts and then re-replacing it from %23 to # at the end of sanitizeMarkdown....

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried reproducing the "removes the char before #", but couldn't... 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I think this might be better

regex: (\W)#(\d)
replacement: $1#​$2

yes, otherwise the chat before will be removed, see screenshot and sample markdown on other PR comment

@viceice viceice added the regression Issue about a regression bug, or the PR caused it label Jul 5, 2023
@setchy
Copy link
Collaborator Author

setchy commented Jul 5, 2023

Adding missing test scenario #23160

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core:changelogs Related to changelogs/release notes fetching regression Issue about a regression bug, or the PR caused it type:bug Bug fix of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants