Skip to content

Conversation

@jens1o
Copy link
Contributor

@jens1o jens1o commented Feb 24, 2017

fixes #312

$docBlock = $node->getAttribute('docBlock');
if ($docBlock !== null) {
return $docBlock->getSummary();
return $docBlock->getSummary() . "\n" . $docBlock->getDescription();
Copy link
Owner

Choose a reason for hiding this comment

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

I think this should be a double line break because it represents a paragraph in markdown and they are separated in the source through a double line break

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, thanks. But why did the tests fail?

@jens1o
Copy link
Contributor Author

jens1o commented Feb 24, 2017

I found out why the tests fail. We should test first wether a description exists, and only then we should add the line breaks.

@jens1o
Copy link
Contributor Author

jens1o commented Feb 24, 2017

Okey, looks good now. But we need to rewrite the tests. Can I do it for you?

@felixfbecker
Copy link
Owner

Of course you can

$docBlock = $node->getAttribute('docBlock');
if ($docBlock !== null) {
return $docBlock->getSummary();
if (empty($docBlock->getDescription()->render())) {
Copy link
Owner

Choose a reason for hiding this comment

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

I would have assumed getDescription() to return null, is that not the case?
the getter and render() should only be called once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. I also wondered about it, but I've tested it:

<?php

require_once 'vendor/autoload.php';

$factory  = \phpDocumentor\Reflection\DocBlockFactory::createInstance();

$docComment = <<<DOCCOMMENT
/**
 * This is an example of a summary.
 */
DOCCOMMENT;

$docblock = $factory->create($docComment);

var_dump($docblock->getDescription()); // returns a Description class with `$bodyTemplate` => "" (string)

// with the description
$description = $docBlock->getDescription()->render();

if (empty(($description))) {
Copy link
Owner

Choose a reason for hiding this comment

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

double parenthesis

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, ran over from a previous, inline version.

@jens1o
Copy link
Contributor Author

jens1o commented Feb 24, 2017

I'd fixed some tests, but there are three, where I don't know what to do. Could you point me to the good direction?

Edit: The online Travis doesn't show that failures... anyway.

@codecov
Copy link

codecov bot commented Feb 24, 2017

Codecov Report

Merging #315 into master will decrease coverage by 1.8%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master     #315      +/-   ##
============================================
- Coverage     85.98%   84.18%   -1.81%     
- Complexity      717      812      +95     
============================================
  Files            53       59       +6     
  Lines          1463     1720     +257     
============================================
+ Hits           1258     1448     +190     
- Misses          205      272      +67
Impacted Files Coverage Δ Complexity Δ
src/DefinitionResolver.php 81.85% <100%> (+0.55%) 276 <0> (+1) ⬆️
src/Server/Workspace.php 68.96% <0%> (-19.27%) 30% <0%> (+6%)
src/ComposerScripts.php 0% <0%> (ø) 10% <0%> (+10%) ⬆️
src/Index/ProjectIndex.php 100% <0%> (ø) 4% <0%> (?)
src/Protocol/FileEvent.php 100% <0%> (ø) 1% <0%> (?)
src/Indexer.php 63.01% <0%> (ø) 18% <0%> (?)
...rc/ContentRetriever/FileSystemContentRetriever.php 100% <0%> (ø) 1% <0%> (?)
src/PhpDocumentLoader.php 100% <0%> (ø) 12% <0%> (?)
src/LanguageServer.php 92.77% <0%> (ø) 20% <0%> (?)
src/Server/TextDocument.php 98.11% <0%> (+0.09%) 58% <0%> (+22%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 56bd465...bbb74d1. Read the comment docs.

laboris commodo ad commodo velit mollit qui non officia id. Nulla duis veniam
veniam officia deserunt et non dolore mollit ea quis eiusmod sit non. Occaecat
consequat sunt culpa exercitation pariatur id reprehenderit nisi incididunt Lorem
sint. Officia culpa pariatur laborum nostrud cupidatat consequat mollit.',
Copy link
Owner

Choose a reason for hiding this comment

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

please don't do un-indented multi-line strings. They break code folding in editors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so I should qoute any line and add a newline at the end?

Copy link
Owner

Choose a reason for hiding this comment

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

yep

@jens1o
Copy link
Contributor Author

jens1o commented Mar 19, 2017

any news on this?

Copy link
Owner

@felixfbecker felixfbecker left a comment

Choose a reason for hiding this comment

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

Sorry, thanks for the ping.


if (empty($description)) {
return $docBlock->getSummary();
} else {
Copy link
Owner

Choose a reason for hiding this comment

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

Remove the else

laboris commodo ad commodo velit mollit qui non officia id. Nulla duis veniam
veniam officia deserunt et non dolore mollit ea quis eiusmod sit non. Occaecat
consequat sunt culpa exercitation pariatur id reprehenderit nisi incididunt Lorem
sint. Officia culpa pariatur laborum nostrud cupidatat consequat mollit.'
Copy link
Owner

Choose a reason for hiding this comment

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

Fix indendation (for code folding)

@jens1o
Copy link
Contributor Author

jens1o commented Mar 19, 2017

like this?

@felixfbecker felixfbecker merged commit 4d0a0a2 into felixfbecker:master Mar 19, 2017
@jens1o jens1o deleted the jens1o-doc-comment branch March 19, 2017 11:34
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.

Hover shows just the very first doc comment paragraph

2 participants