Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/Core/Command/ClosureCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ public function __construct(Closure $closure)
* @param mixed[] $args
* @return mixed
*/
public function __invoke(...$args) {
public function __invoke(...$args)
{
$closure = $this->closure;
return $closure(...$args);
}
Expand Down
12 changes: 11 additions & 1 deletion lib/WorkDoneProgress/MessageProgressNotifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,17 @@ public function begin(
?int $percentage = null,
?bool $cancellable = null
): void {
$this->api->info($message);
$progress = [
$title
];
Comment on lines +45 to +47

@camilledejoye camilledejoye Sep 20, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure we should include the title.
I think it's ment to have a title in a notification window, when showing a message it might get redundant.
With the example of the indexing we will get something like Indexing: Indexing X files

But I admit I went to quickly on the implementation...
For the report I should have handled the cases when the message and or the percentage are not provided.
We could have result like - 10%, which is not really good.
And we might want to not send any message if we don't provide any of this two parameters.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I will try to improve it on this branch and push a proposition, you'll decide then if you keep it or not :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Silly me, I forgot I can't work on this branch directly 😄
camilledejoye@40f222c


if ($message) {
$progress[] = sprintf(': %s', $message);
}
if ($percentage) {
$progress[] = sprintf(', %d%% done', $percentage);
}
$this->api->info(implode('', $progress));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public function testNotifyWithoutWorkDoneProgressCapability(): void
$message = $this->transmitter->shiftNotification();
self::assertEquals('window/showMessage', $message->method);
self::assertEquals(MessageType::INFO, $message->params['type']);
self::assertEquals('begin message', $message->params['message']);
self::assertEquals('title: begin message', $message->params['message']);

$notifier->report($token, 'report message', 30);
$message = $this->transmitter->shiftNotification();
Expand Down
51 changes: 51 additions & 0 deletions tests/Unit/WorkDoneProgress/MessageProgressNotifierTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

namespace Phpactor\LanguageServer\Tests\Unit\WorkDoneProgress;

use PHPUnit\Framework\TestCase;
use Phpactor\LanguageServer\Core\Server\ClientApi;
use Phpactor\LanguageServer\Core\Server\ResponseWatcher\TestResponseWatcher;
use Phpactor\LanguageServer\Core\Server\RpcClient\TestRpcClient;
use Phpactor\LanguageServer\Core\Server\Transmitter\TestMessageTransmitter;
use Phpactor\LanguageServer\WorkDoneProgress\MessageProgressNotifier;
use Phpactor\LanguageServer\WorkDoneProgress\ProgressNotifier;
use Phpactor\LanguageServer\WorkDoneProgress\WorkDoneToken;

class MessageProgressNotifierTest extends TestCase
{
/**
* @var TestMessageTransmitter
*/
private $transmitter;
/**
* @var TestRpcClient
*/
private $api;

protected function setUp(): void
{
$this->transmitter = new TestMessageTransmitter();
$this->api = new TestRpcClient($this->transmitter, new TestResponseWatcher());
}

public function testBegin(): void
{
$token = WorkDoneToken::generate();
$this->createNotifier()->begin($token, 'Hello');
self::assertEquals(1, $this->transmitter->count());
self::assertEquals('Hello', $this->transmitter->shiftNotification()->params['message']);
}

public function testBeginMessageAndPercentage(): void
{
$token = WorkDoneToken::generate();
$this->createNotifier()->begin($token, 'Indexer', 'this may take some time', 50);
self::assertEquals(1, $this->transmitter->count());
self::assertEquals('Indexer: this may take some time, 50% done', $this->transmitter->shiftNotification()->params['message']);
}

private function createNotifier(): ProgressNotifier
{
return new MessageProgressNotifier(new ClientApi($this->api));
}
}