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

bug(#266): sparse-free-decoration in application #285

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

Conversation

h1alexbel
Copy link
Contributor

In this PR, I've updated sparse-decoration.xsl so that it doesn't issue defects in the object application.

see #266
History:

@h1alexbel
Copy link
Contributor Author

@yegor256 please check

@yegor256
Copy link
Member

@maxonfjvipon please, check this one

@yegor256 yegor256 requested a review from maxonfjvipon January 30, 2025 09:00
@maxonfjvipon
Copy link
Member

@h1alexbel I suggest we put this PR on hold until we resolve this. There are some breaking changes which may affect this PR

@h1alexbel
Copy link
Contributor Author

@maxonfjvipon synced PR with EO 0.51.1. Take a look, please

<xsl:text>Sparse decoration is prohibited</xsl:text>
</xsl:element>
<xsl:for-each select="//o">
<xsl:if test="(count(descendant::o[not(@base='Q.org.eolang.bytes')])=1 and o[@name='@'] and (not(@base) or @base!='^') and not(o[@base='∅'])) or count(o[@name='@' and o[@name='@']])&gt;0">
Copy link
Member

@maxonfjvipon maxonfjvipon Jan 31, 2025

Choose a reason for hiding this comment

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

@h1alexbel the idea of sparse decoration is simple - don't use abstract objects if you can.
Consider the example:

[] > foo
  5 > @

Here we can get rid of abstract object:

5 > foo

The same story with:

[] > foo
  x y z > @

It'll be

x y z > foo

But here we can't just get rid of it because of void attribute:

[x] > foo
  5 > @

The same story here:

[] > foo
  5 > @
  4 > x

So the main idea - if abstract object has only one attribute and it has name @ - it's sparse decoration.

Considering this let's simplify the whole condition with something like this:

//o[eo:abstract(.) and count(o)=1 and o[1][@name='@']]

I believe it should cover all the possible cases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maxonfjvipon How about these cases:

# No comments.
[] > test
  # No comments.
  [] > @
    if. > @
      a
      42
      88

I think, sparse-decoration should warn about test only here (@ should be good).

And this:

# No comments.
[] > test
  if. > @
    a
    42
    88

Here, no warnings should be issued. If this is correct, I propose the following XPath for detecting sparse decorations:

//o[(count(descendant::o[not(@base='Q.org.eolang.bytes')])=1 and o[@name='@'] and (not(@base) or @base!='^') and not(o[@base='∅'])) or count(o[@name='@' and o[@name='@']])&gt;0]

This will handle cases above. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

@h1alexbel check this, let's wait for the answer

Copy link
Member

Choose a reason for hiding this comment

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

@h1alexbel here's more about sparse-decoration

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.

3 participants