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

mandatory comments for all abstract objects #921

Closed
yegor256 opened this issue Jul 26, 2022 · 21 comments
Closed

mandatory comments for all abstract objects #921

yegor256 opened this issue Jul 26, 2022 · 21 comments
Assignees
Milestone

Comments

@yegor256
Copy link
Member

yegor256 commented Jul 26, 2022

Let's add a new XSL check into eo-parser that will verify whether all abstract objects have comments prepending them. The severity level should be warning.

Also, let's make sure the comment:

  • starts with a capital letter
  • ends with a dot
  • includes only ASCII printable characters (0x20-0x7f)
  • is at least 64 characters long
  • is in Markdown
  • is grammatically correct :)
@yegor256
Copy link
Member Author

@mximp please, take care of this (or delegate)

@mximp
Copy link
Contributor

mximp commented Aug 8, 2022

@kerelape can you please have a look at this?

@mximp mximp assigned kerelape and unassigned mximp Aug 8, 2022
@yegor256
Copy link
Member Author

yegor256 commented Mar 5, 2023

@mximp what's up with this?

@mximp
Copy link
Contributor

mximp commented Mar 6, 2023

@yegor256 It's in work queue.

@Graur Graur added this to the Routine milestone Aug 11, 2023
@maxonfjvipon
Copy link
Member

@yegor256 we don't save comments into XMIR, so we can't check their presence through XSL.

  1. Do we really need it? Because anonymous abstract object can be used as arguments of application
while.
  TRUE
  # Comment here
  [i]
    i > @

As for me, comment looks a bit strange here
2. We can extend comment rule in our grammar, but ANTLR will allow us to check only:

  • starts with a capital letter
  • ends with a dot
  • includes only ASCII printable characters (but I'm not sure)

WDYT?

@yegor256
Copy link
Member Author

@maxonfjvipon maybe we can prohibit comments for anonymous formations ([x y z]) and make them mandatory for named formations ([x y z] > t)?

@yegor256
Copy link
Member Author

@maxonfjvipon can we check that each comment is a valid Markdown?

@maxonfjvipon
Copy link
Member

@yegor256

maybe we can prohibit comments for anonymous formations ([x y z]) and make them mandatory for named formations ([x y z] > t)?

So even if named abstract object is used as argument for application - it must have comment?

while.
  TRUE
  # Mandatory comment here
  [i] > body
     ...

while.
  TRUE
  [i]   # <-- no comment here
    ...

@yegor256
Copy link
Member Author

@maxonfjvipon yes, correct (such a case will be extremely rare)

@maxonfjvipon
Copy link
Member

@yegor256

can we check that each comment is a valid Markdown?

I'm afraid we can't do it while parsing itself. But I think we can do it while listening in XeEoListener. There we can also check the length of the comment, printable characters

@maxonfjvipon
Copy link
Member

@yegor256 what about horizontal abstract object that may contain inner horizontal abstract objects?

# Mandatory comment
[] ([] (a > b) > y) > x # <--- can't place mandatory comment if front of y

It's the same as:

# Mandatory comment
[] > x
  # Mandatory comment
  [] > y
    a > b

@yegor256
Copy link
Member Author

@maxonfjvipon in this case we may let them skip the comment

@maxonfjvipon
Copy link
Member

@yegor256 I think we should not check the length of the comment before abstract object. Why:

  1. Object may have a complex logic and explanation of its work may take many words and symbols, for example cage
  2. We won't be able to leave todos in EO code
  3. We won't be able to "inlined EO inserts" in the comment in order to show how the object works (in far future)

So I suggest not to check the length of the comment at all. WDYT?

@yegor256
Copy link
Member Author

@maxonfjvipon you mean, we should not require the comments to have at least 64 symbols, as originally suggested?

@maxonfjvipon
Copy link
Member

@yegor256 oh, there is "at least" (>= 64). Then it's ok, sorry

@maxonfjvipon
Copy link
Member

maxonfjvipon commented Jan 31, 2024

@yegor256 maybe reduce this rule to "at least 32 chars"? 64 seems to be quite many

@yegor256
Copy link
Member Author

@maxonfjvipon it's better to start with as strict requirement as we can. If we start with 32, it will be impossible to make it larger later, since many programs will be written already.

maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
maxonfjvipon added a commit to maxonfjvipon/eo that referenced this issue Jan 31, 2024
@0pdd
Copy link

0pdd commented Feb 1, 2024

@yegor256 the puzzle #2835 is still not solved.

@0pdd
Copy link

0pdd commented Feb 2, 2024

@yegor256 the puzzle #2841 is still not solved; solved: #2835.

@0pdd
Copy link

0pdd commented Feb 5, 2024

@yegor256 the puzzle #2860 is still not solved; solved: #2835, #2841.

@yegor256
Copy link
Member Author

yegor256 commented Mar 9, 2024

@maxonfjvipon I believe it's fixed

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

No branches or pull requests

6 participants