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

Allow configurable file extension for indexing #308

Open
wants to merge 31 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
cdb5b56
Add support to index multiple file extensions
Feb 18, 2017
5f096c4
Add test for indexing multiple file types
Feb 18, 2017
7dc4477
Fix wrong phpDoc type
Feb 18, 2017
f7175bc
Filter invalid file types and use default list as fallback
Feb 18, 2017
9433694
Let JsonMapper intialize the options
Feb 18, 2017
39cfbda
Add test for fileTypes option
Feb 18, 2017
3c33e7f
Initialize options with default values when not provided by client
Feb 18, 2017
d2e5048
Update testIndexingMultipleFileTypes
Feb 18, 2017
b9d0d1b
Add missing namespace in OptionsTest
Feb 18, 2017
9067b44
Fix wrong classname for options test
Feb 18, 2017
1e319c7
Wipe index when on configuration change
Feb 24, 2017
58c82e6
Add list of valid indexer options
Mar 2, 2017
44a942e
Implement didChangeConfiguration event
Mar 2, 2017
940eb97
Pass options and indexer to workspace
Mar 2, 2017
5b1b6bf
Add tests
Mar 2, 2017
707c97f
Merge branch 'master' of github.com:felixfbecker/php-language-server …
Mar 2, 2017
1e73d08
Improve gettting changed options
Mar 4, 2017
1f90b4e
Update options one by one to update all instance
Mar 4, 2017
ca225ff
Remove emitting wipe events
Mar 4, 2017
c4568bf
Accept different types/formats from clients
Mar 4, 2017
5308e7a
Add new tests and update old ones
Mar 4, 2017
a06057b
Fix phpcs warnings/errors
Mar 4, 2017
23a40f0
Let didChangeConfiguration decide what options are interesting for th…
Mar 4, 2017
f4f1067
Change didChangeConfiguration doc to protocol wording
Mar 4, 2017
9cc2736
Merge remote-tracking branch 'upstream/master' into feature/allow-con…
JSteitz Aug 29, 2018
09fbec2
Refactor pull request
JSteitz Aug 29, 2018
a5417cd
Fix risky test warning
JSteitz Aug 29, 2018
e317e8c
Start indexing after initialization
JSteitz Aug 31, 2018
a1c3845
WIP: Implement didChangeConfiguration with reindexing
JSteitz Aug 31, 2018
a1e5654
WIP: Implement didChangeConfiguration with reindexing
JSteitz Aug 31, 2018
a81bed9
Partial work on feedback
JSteitz Aug 31, 2018
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
1 change: 1 addition & 0 deletions fixtures/different_extension.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<?php
21 changes: 21 additions & 0 deletions src/Client/Workspace.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
namespace LanguageServer\Client;

use LanguageServer\ClientHandler;
use LanguageServer\Protocol\ConfigurationItem;
use LanguageServer\Protocol\TextDocumentIdentifier;
use Sabre\Event\Promise;
use JsonMapper;
Expand Down Expand Up @@ -44,4 +45,24 @@ public function xfiles(string $base = null): Promise
return $this->mapper->mapArray($textDocuments, [], TextDocumentIdentifier::class);
});
}

/**
* The workspace/configuration request is sent from the server to the
* client to fetch configuration settings from the client.
*
* The request can fetch n configuration settings in one roundtrip.
* The order of the returned configuration settings correspond to the order
* of the passed ConfigurationItems (e.g. the first item in the response is
* the result for the first configuration item in the params).
*
* @param ConfigurationItem[] $items
* @return Promise
Copy link
Owner

Choose a reason for hiding this comment

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

What's the type of the Promise?

*/
public function configuration(array $items): Promise
{
return $this->handler->request(
'workspace/configuration',
['items' => $items]
);
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't this be mapped to a class?

}
}
10 changes: 10 additions & 0 deletions src/Index/AbstractAggregateIndex.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,4 +147,14 @@ public function getReferenceUris(string $fqn): array
}
return $refs;
}

/**
* Wipe all indexes for a reindex
*/
public function wipe()
{
foreach ($this->getIndexes() as $index) {
$index->wipe();
}
}
}
11 changes: 11 additions & 0 deletions src/Index/Index.php
Original file line number Diff line number Diff line change
Expand Up @@ -222,4 +222,15 @@ public function serialize()
'staticComplete' => $this->staticComplete
]);
}

/**
* Clear indexed references and definitions
*/
public function wipe()
{
$this->definitions = [];
$this->references = [];
$this->complete = false;
$this->staticComplete = false;
}
}
2 changes: 1 addition & 1 deletion src/Index/ProjectIndex.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public function __construct(Index $sourceIndex, DependenciesIndex $dependenciesI
/**
* @return ReadableIndex[]
*/
protected function getIndexes(): array
public function getIndexes(): array
{
return [$this->sourceIndex, $this->dependenciesIndex];
}
Expand Down
72 changes: 70 additions & 2 deletions src/Indexer.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ class Indexer
*/
private $documentLoader;

/**
* @var Options
*/
private $options;

/**
* @var \stdClasss
*/
Expand All @@ -63,6 +68,16 @@ class Indexer
*/
private $composerJson;

/**
* @var bool
*/
private $hasCancellationSignal;

/**
* @var bool
*/
private $isIndexing;

/**
* @param FilesFinder $filesFinder
* @param string $rootPath
Expand Down Expand Up @@ -93,6 +108,22 @@ public function __construct(
$this->documentLoader = $documentLoader;
$this->composerLock = $composerLock;
$this->composerJson = $composerJson;
$this->hasCancellationSignal = false;
$this->isIndexing = false;
$this->options = new Options();
}

/**
* @param Options $options
*/
public function setOptions(Options $options)
{
$this->options = $options;
}

public function getOptions(): Options
{
return $this->options;
}

/**
Expand All @@ -103,13 +134,14 @@ public function __construct(
public function index(): Promise
{
return coroutine(function () {

$pattern = Path::makeAbsolute('**/*.php', $this->rootPath);
$fileTypes = implode(',', $this->options->fileTypes);
$pattern = Path::makeAbsolute('**/*{' . $fileTypes . '}', $this->rootPath);
felixfbecker marked this conversation as resolved.
Show resolved Hide resolved
$uris = yield $this->filesFinder->find($pattern);

$count = count($uris);
$startTime = microtime(true);
$this->client->window->logMessage(MessageType::INFO, "$count files total");
$this->isIndexing = true;

/** @var string[] */
$source = [];
Expand All @@ -135,6 +167,7 @@ public function index(): Promise
$this->client->window->logMessage(MessageType::INFO, 'Indexing project for definitions and static references');
yield $this->indexFiles($source);
$this->sourceIndex->setStaticComplete();

// Dynamic references
$this->client->window->logMessage(MessageType::INFO, 'Indexing project for dynamic references');
yield $this->indexFiles($source);
Expand Down Expand Up @@ -187,6 +220,7 @@ public function index(): Promise
}
}

$this->isIndexing = false;
$duration = (int)(microtime(true) - $startTime);
$mem = (int)(memory_get_usage(true) / (1024 * 1024));
$this->client->window->logMessage(
Expand All @@ -196,6 +230,35 @@ public function index(): Promise
});
}

/**
* Return current indexing state
*
* @return bool
*/
public function isIndexing(): bool
{
return $this->isIndexing;
}

/**
* Cancel all running indexing processes
*
* @return Promise
*/
public function cancel(): Promise
{
return coroutine(function () {
$this->hasCancellationSignal = true;

while ($this->isIndexing()) {
yield timeout();
}
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not that happy with this "polling" solution - usually cancellation is synchronous and best-effort. I.e. the old indexing would stop eventually but you can already start with the next index run, by just making sure to instantiate new Index objects (maybe even a new Indexer) so that the old run can't accidentally still write to the new index. Does that sound feasible?

Copy link
Author

Choose a reason for hiding this comment

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

Don't think a new Indexer instance is a good idea, they are used in 2 different places (initialized and didChangeConfiguration) and I would need a reference to both.

And I think creating new Index objects is too much work from what I see.
Are you talking about sourceIndex and dependencyIndex?

If yes, then the method would need to know about all objects that require the index objects. Or am I wrong here?


$this->hasCancellationSignal = false;
$this->client->window->logMessage(MessageType::INFO, 'Indexing project canceled');
});
}

/**
* @param array $files
* @return Promise
Expand All @@ -204,6 +267,11 @@ private function indexFiles(array $files): Promise
{
return coroutine(function () use ($files) {
foreach ($files as $i => $uri) {
// abort current running indexing
if ($this->hasCancellationSignal) {
return;
}

// Skip open documents
if ($this->documentLoader->isOpen($uri)) {
continue;
Expand Down
Loading