-
Notifications
You must be signed in to change notification settings - Fork 166
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
initial docstring support #515
base: master
Are you sure you want to change the base?
Conversation
Hi bagel, thank you for the contribution. Since this database is actually really just a "cache"/"index" of the files on disk, I think we don't really need a full blown migration. So, rather than checking whether the database columns matches, we can just have a table to store metadata for the database file, e.g. the version of rope that created the database. If the rope version doesn't match, then just wipe the tables. For convenience during development of rope itself, where version number may not reflect actual on-disk changes, we can just wipe the database when the hashes of the files in rope/contrib/autoimport directory changes, but I think even a manual wipe with |
Good point - the tests are in memory so they should work but I'll look into the version table |
Hi Bagel, thanks for this, the changes for collecting documentation looks reasonable to me. However, I'm not quite seeing the reason why it is necessary to index the docstrings, maybe you can help me understand here. Autocompletions need to index names because it needs fast search of the python names as the user types, but once you get the possible matches, wouldn't it be possible to just retrieve the docstring when asked by the editor in real time rather than preemptively returning all docstrings? For example, LSP >= 3.16.0 allows lazy resolution of an autocompleted name's documentation. Resolving in real-time would allow for more complex resolution strategies as well that wouldn't really be easy during indexing time, for example, if a name is aliased or re-exported by another module, or you can retrieve the documentation for global object instances by inferring the type of the object and finding the docstring of its class. |
That is a valid point - this can't get more than like 40% of docstrings so an import based solution would be better for these. On the other hand, I think the increased DB size and time isn't too bad, so it may be worth it to add it here as a baseline and let other projects build atop it (perhaps they could import if its missing/blank?). Also importing has the arbitrary code execution issue while this is AST based. |
eb8e5fb
to
6476478
Compare
commit c4b7ada05a63c92c94e17cd6383c90284a16b820 Author: bagel897 <[email protected]> Date: Wed Jan 10 11:16:51 2024 -0600 reformat commit eb8e5fb Author: bagel897 <[email protected]> Date: Mon Nov 7 23:17:15 2022 -0600 Clean docs, add async support commit a07151a Merge: 1f0556c 764bbe1 Author: Bagel <[email protected]> Date: Thu Nov 3 20:46:31 2022 -0500 Merge branch 'master' into autoimport_docstring commit 1f0556c Author: bagel897 <[email protected]> Date: Thu Nov 3 01:52:32 2022 -0500 Expose the modname commit 3788d54 Author: bagel897 <[email protected]> Date: Thu Nov 3 00:52:52 2022 -0500 initial docstring support
8776c54
to
3099c2a
Compare
#504
BREAKING CHANGES:
Other concerns:
Preview
Side note: I really love the ORM you made, its very cool
Checklist (delete if not relevant):