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

feat(#343): POC: show defect context if @line attributes are empty #349

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

h1alexbel
Copy link
Contributor

In this PR I've implemented Proof Of Concept of printing defect's context, if inspected XMIR doesn't have @line attributes inside.

see #343
History:

@h1alexbel
Copy link
Contributor Author

@volodya-lombrozo take a look, please

Copy link
Member

@volodya-lombrozo volodya-lombrozo left a comment

Choose a reason for hiding this comment

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

@h1alexbel Thank you for the contribution! Just one small comment.
Additionally, maybe we can add a puzzle that would suggest to fix the issue with large context values? What do you think?


/**
* Defect with context.
* @since 0.0.0
Copy link
Member

Choose a reason for hiding this comment

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

@h1alexbel [NIT] Can we put exact numbers here 0.1.0 or 0.0.40 ?

@h1alexbel
Copy link
Contributor Author

@h1alexbel Thank you for the contribution! Just one small comment. Additionally, maybe we can add a puzzle that would suggest to fix the issue with large context values? What do you think?

@volodya-lombrozo for lage-context values, we already show only first 300 chars from the context string. In defect-context.xsl we do:

<xsl:value-of select="substring(serialize($context, map{'method':'xml'}), 1, 300)"/>

@volodya-lombrozo
Copy link
Member

@h1alexbel Thank you for the contribution! Just one small comment. Additionally, maybe we can add a puzzle that would suggest to fix the issue with large context values? What do you think?

@volodya-lombrozo for lage-context values, we already show only first 300 chars from the context string. In defect-context.xsl we do:

<xsl:value-of select="substring(serialize($context, map{'method':'xml'}), 1, 300)"/>

@h1alexbel Could you add the test for this case please?

@h1alexbel
Copy link
Contributor Author

@volodya-lombrozo updated. Take a look, please

Copy link
Member

@volodya-lombrozo volodya-lombrozo left a comment

Choose a reason for hiding this comment

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

@h1alexbel Looks good to me, thank you!

@h1alexbel
Copy link
Contributor Author

@yegor256 take a look, please, what do you think about such feature?

@h1alexbel
Copy link
Contributor Author

@yegor256 reminder

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