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

Multiple Constant declaration and documentation without newlines get ignored #798

Closed
dfherr opened this issue Sep 15, 2014 · 8 comments
Closed

Comments

@dfherr
Copy link

dfherr commented Sep 15, 2014

If i declare multiple constants in consecutive lines and try to document them with multiple spaces and a comment on the same line yard ignores them.

Yard does not seem to ignore them if i put linebreaks between each constant or the constant is directly followed by 1 space and the constant documentation.

This behavior is kind of weird and i think yard should handle that correctly.

Here is an example which didn't work for me:

     # constants for row columns for better readability
      REFERENCE = 0          # column A, occurances (reference)
      COMMENT = 1            # column B, comment
      MSGCTXT = 2            # column C, id (msgctxt)
      MSGID = 3              # column D, original (msgid)
      MSGSTR = 4             # column E, translation (msgstr)

These are helper constants for corresponding gettext columsn in a 2d array. I want the comments to be easily readable by the human eye and also yardoc to document them.

@lsegal
Copy link
Owner

lsegal commented Sep 15, 2014

YARD is behaving as expected here. YARD groups comment blocks that start on the same column together into one comment. There are a couple of problems here:

  1. The first line comment is being associated with REFERENCE because YARD always chooses the line above over the comment to the right. You can separate the comment above with TWO lines of spacing:

    # actual docstring here
    CONST1 = 1 # not used
    
    # not the docstring because of newlines
    
    
    CONST2 = 1 # actual docstring
    
  2. You can't group comments on the same column. YARD will think they are a single comment. This is because Ruby has no multiline commenting form, so YARD emulates this by detecting multiline comments by their start column.

This works:

module Core
  # constants for row columns for better readability


  REFERENCE = 0          # column A, occurances (reference)

  COMMENT = 1            # column B, comment

  MSGCTXT = 2            # column C, id (msgctxt)

  MSGID = 3              # column D, original (msgid)

  MSGSTR = 4             # column E, translation (msgstr)
end

Though at that point my recommendation would just be to put the comments on top instead of on the right.

@dfherr
Copy link
Author

dfherr commented Sep 15, 2014

i'm not sure if i do not understand you correctly or I didn't make myself clear.

What i want is that each constant is associated with the comment on the right (if there is no comment in the line above, so i'm omitting that from the previous example as you are certainly right about that)

Example 1
This does not work for me and does not document any constant. yard stats --list-undoc will show these constants as undocumented. I think this should work though.

      REFERENCE = 0          # column A, occurances (reference)
      COMMENT = 1            # column B, comment
      MSGCTXT = 2            # column C, id (msgctxt)
      MSGID = 3              # column D, original (msgid)
      MSGSTR = 4             # column E, translation (msgstr)

Example 2
This does work as expected.

     # column A, occurances (reference)
     REFERENCE = 0,
     # column B, comment    
     COMMENT = 1  
     # column C, id (msgctxt)      
     MSGCTXT = 2 
     # column D, original (msgid)        
     MSGID = 3 
     # column E, translation (msgstr)             
     MSGSTR = 4           

Example 3
As does this

      REFERENCE = 0          # column A, occurances (reference)

      COMMENT = 1            # column B, comment

      MSGCTXT = 2            # column C, id (msgctxt)

      MSGID = 3              # column D, original (msgid)

      MSGSTR = 4             # column E, translation (msgstr)

Example 4
And it also works if i only seperate each constant from the comment with just one space

      REFERENCE = 0 # column A, occurances (reference)
      COMMENT = 1 # column B, comment
      MSGCTXT = 2 # column C, id (msgctxt)
      MSGID = 3 # column D, original (msgid)
      MSGSTR = 4 # column E, translation (msgstr)

So why does Example 4 work but 1 does not? That is confusing behavior and i do not get why that should be intended.

Edit: after reading your comment again i finally got what you meant about grouping in columns. It seems reasonable, but couldn't that be changed for situations like this where this behavior is obviously wrong? Why should someone group a multiline comment in front of variable declaration that is meant to be documented to a single variable?

Grouping on the same column could be interupted on the next documentable declaration. e.g. split groups on next module, class, method or constant declaration.

@lsegal
Copy link
Owner

lsegal commented Sep 15, 2014

Example 4 doesn't work exactly the way you think it does (COMMENT will have its docstring taken by MSGTXT because both comments start on the same column). But that's probably not the point you were trying to make...

Example 1 doesn't work because all of your comment lines are treated as a single multi-line comment block when they start on the same column and have no whitespace separating them. There is a simple heuristic for determining when to associate a comment block or single line comment to an object:

  1. Does the comment block END on the same line as the object?
  2. Does the comment block END on the line before the object?
  3. Does the comment block END 2 lines before the object?

If any of those conditions are true, the comment is associated with the object. In Example 1, two distinct behaviors occurred to create the result you are seeing:

  1. Your 5 comments were merged into a single comment block because they all start on the same column.
  2. The merged comment block ended on the MSGSTR object line, therefore matching the condition to associate the single comment block with that object and no other.

Right now this is expected behavior. Given that RDoc doesn't even handle end-of-line comments, this may not be perfect, but it's a step up above existing behavior. There are no plans to change this behavior, but if you want to dig into the parsing code and submit a pull request that can intelligently split up these comments, I would consider pulling it in.

@dfherr
Copy link
Author

dfherr commented Sep 15, 2014

I will have a look at the code and consider if I could do it in reasonable time.

Where should be a line in the block comment detection that stops if it found an object associated to the block and on the next line is another object.

edit: it's really too late here to post on github, i should look into it again tomorrow...

@lsegal
Copy link
Owner

lsegal commented Sep 15, 2014

That single change to the heuristic wouldn't solve the problem. The underlying issue is that all the comment lines are grouped together into a single block. That happens before the heuristic is applied. You'll still get a scenario where only one of the constants gets the full block. I only listed the heuristic to explain why it was the MSGSTR constant that got the block and not the others.

@dfherr
Copy link
Author

dfherr commented Sep 16, 2014

yea i realised that too and tried to edit my comment before you answer :D was really too late yesterday

@lsegal lsegal closed this as completed in ba02432 Oct 26, 2014
@lsegal
Copy link
Owner

lsegal commented Oct 26, 2014

@dfherr I made a change to the ripper parser heuristic that does not group comments if they succeed actual code on the line. In other words:

foo # comment1
bar # comment 2

Should never be considered a multiline comment of "comment 1\ncomment2".

Note two distinctions, however:

  1. This will not work in the legacy parser. That should typically not be an issue anymore.
  2. This does not handle the first line comment preceding the entire const block in your example. The heuristic to prioritize the comment above the object first has not changed. In other words, YARD searches for a comment on (line-1), followed by (line-2), followed by (line). That needs to stay the way it is to deal with the following common convention:
# Comments for Foo
class Foo # :nodoc:
...

Constants have a different behavior, but unfortunately we have not yet detected that the line in question is a "constant", so there's no way to change the behavior here. Your example would now work with the minor change:

# constants for row columns for better readability


REFERENCE = 0          # column A, occurances (reference)
COMMENT = 1            # column B, comment
MSGCTXT = 2            # column C, id (msgctxt)
MSGID = 3              # column D, original (msgid)
MSGSTR = 4             # column E, translation (msgstr)

@dfherr
Copy link
Author

dfherr commented Nov 28, 2014

awesome, thank you :)

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

No branches or pull requests

2 participants