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: MarkdownRemover updates #57

Merged
merged 2 commits into from
Jan 11, 2025

Conversation

chr15k
Copy link
Contributor

@chr15k chr15k commented Jan 9, 2025

Description

  • Condense duplicated regex
  • Regex to remove more than 2 layers of emphasis

Motivation and context

This contribution is for an open-source project I actively use. It aims to enhance the markdown remover functionality by adding support for removing triple emphasis from text and addressing some TODOs noted in the code comments.

How has this been tested?

The Docker README instructions didn’t work out of the box for me because Composer required a minimum of PHP 8.2 but 8.1 was being installed. I updated the default PHP version in the Makefile/docker-compose/Dockerfile to use PHP 8.2. Additionally, I encountered warnings related to the Docker configuration. However, I haven’t committed these changes to a separate PR since I’m unsure whether they are actual fixes or specific to my environment.

Tested using:

make tests-dox

Checklist:

Go over all the following points before making your PR:

  • I have read the CONTRIBUTING document.
  • My pull request addresses exactly one patch/feature.
  • I have created a branch for this patch/feature.
  • I have added tests to cover my changes.
  • If my change requires a change to the documentation, I have updated it accordingly.

If you're unsure about any of these, don't hesitate to ask. We're here to help!

@tigitz
Copy link
Owner

tigitz commented Jan 11, 2025

Thanks for the fix, I've updated the master branch to be up to date with latest php version and deps, could you please rebase ?

@chr15k
Copy link
Contributor Author

chr15k commented Jan 11, 2025

Thanks for the fix, I've updated the master branch to be up to date with latest php version and deps, could you please rebase ?

done

@tigitz tigitz merged commit 88e1d08 into tigitz:master Jan 11, 2025
4 of 5 checks passed
@tigitz
Copy link
Owner

tigitz commented Jan 11, 2025

Thanks ! Congrats for your first contribution 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants