dlang-community / DCD

The D Completion Daemon is an auto-complete program for the D programming language
GNU General Public License v3.0
349 stars 71 forks source link

dcd doesn't work with arsd-official (reportedly) #666

Open adamdruppe opened 2 years ago

adamdruppe commented 2 years ago

I've never used it myself but user complained dcd can't auto-complete dom.d in arsd-official. It probably doesn't scan the directory or import the dub things correctly.

rikkimax commented 2 years ago

This could be an IDE issue (as far as I'm aware DCD itself doesn't know about dub).

WebFreak001 commented 2 years ago

this is fixed since https://github.com/dlang-community/DCD/pull/328 in IDEs that pass in the import files from DUB into DCD.

In code-d/serve-d this has been disabled though as it significantly increased memory usage with some packages. I think some of these packages were vibe-d and arsd-official.

trikko commented 2 years ago

Are you sure that is the same bug?

Setting this:

$ dcd-client -I ~/.dub/packages/arsd-official-10.8.2/arsd-official/

This works:

import      dom;
void a() { Document d; d. }
$ dcd-client -l -c 42 source/app.d 
/home/andrea/.dub/packages/arsd-official-10.8.2/arsd-official/dom.d 2684

This doesn't work:

import arsd.dom;
void a() { Document d; d. }
$ dcd-client -l -c 42 source/app.d 
Not found

But if I move arsd-official to /tmp/arsd and I set:

dcd-client -I /tmp/arsd

It works.

It seems that dcd doesn't like package names that don't match dir name.

$ grep "^module" /tmp/arsd/dom.d
module arsd.dom;
rikkimax commented 2 years ago

Ah huh!

This has something to do with Adams usage of -mv.

On Mon, May 16, 2022, 19:28 Andrea Fontana @.***> wrote:

Are you sure that is the same bug?

Setting this:

$ dcd-client -I ~/.dub/packages/arsd-official-10.8.2/arsd-official/

This works:

import dom;void a() { Document d; d. }

$ dcd-client -l -c 42 source/app.d /home/andrea/.dub/packages/arsd-official-10.8.2/arsd-official/dom.d 2684

This doesn't work:

import arsd.dom;void a() { Document d; d. }

$ dcd-client -l -c 42 source/app.d Not found

But if I move arsd-official to /tmp/arsd and I set:

dcd-client -I /tmp/arsd

It works.

It seems that dcd doesn't like package names that don't match dir name.

$ grep "^module" /tmp/arsd/dom.d module arsd.dom;

— Reply to this email directly, view it on GitHub https://github.com/dlang-community/DCD/issues/666#issuecomment-1127323400, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHSL4ZLXVPDRG3PUMWSERLVKH2J3ANCNFSM5V7IGGGQ . You are receiving this because you commented.Message ID: @.***>

WebFreak001 commented 2 years ago

you are right, the change adding support for indexing individual files doesn't change the module matching, it still only relies on the path to resolve which modules are where.

DCD would need to implement a switch like -mv as well and IDEs would need to pass it to DCD to fix this.

adamdruppe commented 2 years ago

You can simply scan the directory and read module names. You can discard everything else except the module mapping until it is actually requested if memory is a problem.

WebFreak001 commented 2 years ago

unnecessary reads of files just to figure out the module names would introduce a lot of I/O overhead for very little benefit as 98% of the dub ecosystem have matching path & module naming and you would need to actively add compiler flags in your dub configurations to make other layouts work properly. (as dmd also looks up files in import paths like that, as long as the files aren't explicitly passed to the command line (in the right order))

Supporting -mv, which is also already inside your package recipe, seems to me like a much better idea, not introducing a lot of unnecessary (potentially slow) I/O calls.

rikkimax commented 2 years ago

A big problem with -mv is that currently, dub does not understand it, and therefore neither do IDE's.

It's going to need its own map directive, and from there translated to compiler + dcd specific flags. If this was implemented, it should be a pretty simple fix on the IDE side via $ dub describe.

adamdruppe commented 2 years ago

98% of the dub ecosystem have matching path & module naming

I actually have matching path and module naming.

As I've said many times, what happens when you git clone git@github.com:adamdruppe/arsd.git ?

But whatever, it is all a pile of broken code, so if more broken code has to undo the first bit of broken code, whatever.

trikko commented 2 years ago

Register this adam: https://code.dlang.org/packages/arsd :)

adamdruppe commented 2 years ago

I'd be a p big breakage.....

You could ln -s arsd-official arsd in that directory, then use the -I thing, that'd work to trick dcd into working.

trikko commented 2 years ago

unnecessary reads of files just to figure out the module names would introduce a lot of I/O overhead for very little benefit as 98% of the dub ecosystem have matching path & module naming

Wait a sec, doesn't dcd scan files for parsing definitions just one time when you import them? If it parse the file, it can read the module name inside and use that instead of dir/filename.d.

What am I missing?

trikko commented 2 years ago

I'd be a p big breakage.....

You could ln -s arsd-official arsd in that directory, then use the -I thing, that'd work to trick dcd into working.

You can keep both. Don't kill arsd-official. Just register arsd and link it to the same github repo. Maybe in a 10 years you can depracate arsd-official :)

WebFreak001 commented 2 years ago

we can make it work through the sourceFiles setting in dub, which can be indexed by DCD, (and is also used in a lot of packages which have this kind of file layout) however I strongly oppose reading all the .d files from all the import directories to look for module names.