Index reserved keywords for better user experience #19
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
derickr
requested changes
Jul 30, 2024
gen-phpweb-sqlite-db.php
Outdated
|
|
||
| if ($f === 'reserved.keywords.php') { | ||
| // Yes, this is fragile. | ||
| $handle = fopen($file, 'r'); |
There was a problem hiding this comment.
There is no error handling here.
There was a problem hiding this comment.
What sort of handling would you like to see here? Would it be enough to move this into the above if statement?
There was a problem hiding this comment.
At least not try to run any code if you can't open the file. Right now ,that would error on line 173 with the fgets.
Possibly. What's the plan for keeping the list introduced in php/web-php#804 up-to-date? This change adds functionality with very little burden of maintenance as far as I can tell. |
derickr
requested changes
Aug 1, 2024
gen-phpweb-sqlite-db.php
Outdated
|
|
||
| if ($f === 'reserved.keywords.php') { | ||
| // Yes, this is fragile. | ||
| $handle = fopen($file, 'r'); |
There was a problem hiding this comment.
At least not try to run any code if you can't open the file. Right now ,that would error on line 173 with the fgets.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
URLs like https://www.php.net/strpos or https://php.net/implode render the expected documentation page.
Some URLs are specifically configured to redirect to the expected documentation page, like https://php.net/_GET or https://www.php.net/foreach.
However URLs for language features such as
yield(https://php.net/yield) andendif(https://php.net/endif) redirect to a function search page which claims that the feature does not exist.This pull request adds some logic to read the list of reserved keywords from the relevant page of documentation and updates the index to include these results. This means that all the URLs in this description should work as expected.
Note that if php/web-php#804 gets merged before this, we can probably re-assess if this is necessary. There are some comments in the code about refactoring this to read from phd directly or to have phd generate a search database itself.