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

add support to Log Message breakpoint #427

Merged
merged 2 commits into from
May 30, 2024

Conversation

HenryRiley0
Copy link
Contributor

#426 @WebFreak001, thanks for your advice, i modified the code, makes LogMessage var and function in a class.

@WebFreak001
Copy link
Owner

cool! can you add a sample, maybe a sample screenshot/gif in the README or other docs file to show off this feature?

const matches = this.logMsgVar.match(this.logReplaceTest);
const count = matches ? matches.length : 0;
this.logMsgRplNum = count;
this.logMsgVarProcess = this.logMsgVar.replace(this.logReplaceTest, "placeHolderForVariable");
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not a huge fan of the magic string here and would much prefer if we could instead maybe split by the regex, so it's not possible to have this value accidentally in the output message (even if very unlikely)

@WebFreak001
Copy link
Owner

btw you can just push to the new branch that you made and the PR will automatically update on github, you don't have to create a new one when you change things

2. add logMessage in README.
@HenryRiley0
Copy link
Contributor Author

btw you can just push to the new branch that you made and the PR will automatically update on github, you don't have to create a new one when you change things

thank you. i fix the lint error, add a gif in readme

@WebFreak001 WebFreak001 merged commit f204573 into WebFreak001:master May 30, 2024
5 checks passed
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