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

Polish before publishing #30

Open
jacobdufault opened this Issue Nov 18, 2017 · 4 comments

Comments

Projects
None yet
4 participants
@jacobdufault
Member

jacobdufault commented Nov 18, 2017

Higher priority items I'd like to get addressed before publishing to the marketplace

  • make sure we don't load cached file if real file is not present
  • Very make_unique contributes constructor calls
  • extern const char not included in outline? see ash_switches.h
  • show warning for compile_commands.json, autogen if possible; maybe git rev-parse origin/master ; or git rev-parse HEAD
  • Verify when loading project we add to end of queue so existing open files are indexed first
  • we don't need to reindex file unless it has timestamp change, ever, try to add metrics to count how many files get updated after sync
  • reindex file is command line arguments change
  • max number of code lens results otherwise vscode will hang if the file has 20k+ code lens
  • Mention how to disable code lens in README
  • Drop clang_args in favor of pref or move to .cquery or similar
  • clang arg pref should be platform specific
  • Only status update every few hundred milliseconds
  • Reduce output
  • index perf: don't run every DoIdMap; stop after 500 or 1000 iterations
  • Move completion entirely off querydb thread so it never touches querydb
  • progress indicator needs to account for index threads which are busy (ie, +1 to a job when they remove the item from the queue until it is done being processed)

Misc issues that need to be dealt with at some point

  • Make warning "launch.workingDirectory" say "cquery.launch.workingDirectory"
  • std::sting -> std::string typo fixit does replaces trailing text content/space, it should insert the new string instead of replacing, likely a bug in FixIt generation
  • Fix show references for function declaration parameters
  • Code lens causing screen to scroll - Microsoft/vscode#12294
  • Show warning for old pref names
  • No reload notification when adding new pref that changes from default value (include.blacklist)
  • Find ref on lambda parameter
  • gv on variable should show variables of the same type
  • Freshen should drop existing index
  • labeled stack traces on linux
  • Insert include by typing a symbol (ie, global symbol search)
  • see if we can set code lens command title to show custom string
  • Prefer smallest path found for include when scanning
  • Clickable controls in tooltips - Microsoft/vscode#29076
  • See if we can integrate commit characters for code completion
  • Auto-implement doesn't strip override
  • Auto-implement select implemented content for fast ctrl-x ctrl-v
  • Call tree in context menu
  • Call tree - use icon for base/derived calls
  • Completion list private with - or similar
  • Don't show errors for ie, *.mojom files
  • Add task to reload compile_commands.json
  • Call tree should store index lines and look it up when the user requests goto definition
  • Add task to drop existing index and do a fresh import
  • Reparse top document - might be possible by hooking directly into extension API and sending a notification $cquery/onView
  • Call hierarchy; show class name and line, skip column MyMethod (Foobar:3). If no class name show file MyMethod (foo.cc:3)
  • Call hierarchy: sort by file and then by location in that file
  • Call tree fails on pure-virtual functions
  • Investigate jemalloc
  • Do not report global completions in certain situations, ie, o not report global completions in certain situations, ie, Example: type this: class Foo : public Bar {public:
  • Idea: only do live-indexing after a save, but use completion thread to do it.
@nilbot

This comment has been minimized.

Show comment
Hide comment
@nilbot

nilbot Nov 18, 2017

I really like this list, most items seem not easy so I already feel bad about adding more item to the list. But I really think some degree of user-friendliness is welcomed for a good release, so here we go:

  • document or automatically setup resourceDirectory on different platforms. E.g. for folks who uses Apple's LLVM, point it to /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include
  • consider default setting for workingDirectory to /var/log/cquery or ~/(var|Library)/log/cquery. Currently the diagnostics log goes into this directory. Is this all cquery currently using for?
  • for folks who want (try) to keep their source folder tidy, they might want to put all projects' cache into say ~/var/cache/$project. It would be helpful to have the extension support variables like $workspaceBasename so that they can set "cquery.cacheDirectory" to "${env:HOME_CACHE_ROOT}/${projectBasename}

nilbot commented Nov 18, 2017

I really like this list, most items seem not easy so I already feel bad about adding more item to the list. But I really think some degree of user-friendliness is welcomed for a good release, so here we go:

  • document or automatically setup resourceDirectory on different platforms. E.g. for folks who uses Apple's LLVM, point it to /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include
  • consider default setting for workingDirectory to /var/log/cquery or ~/(var|Library)/log/cquery. Currently the diagnostics log goes into this directory. Is this all cquery currently using for?
  • for folks who want (try) to keep their source folder tidy, they might want to put all projects' cache into say ~/var/cache/$project. It would be helpful to have the extension support variables like $workspaceBasename so that they can set "cquery.cacheDirectory" to "${env:HOME_CACHE_ROOT}/${projectBasename}
@topisani

This comment has been minimized.

Show comment
Hide comment
@topisani

topisani Nov 19, 2017

Contributor

std::sting -> std::string typo fixit does replaces trailing text content/space, it should insert the new string instead of replacing, likely a bug in FixIt generation

In emacs i subtract 1 from the end of all textedits, since emacs uses an inclusive range, and cquery (i assume) uses an exclusive range, since otherwise replacing 0 characters, ie. just inserting wouldnt be possible.

Edit:
Turns out thats actually not the right thing to do - think the cquery server allways points to an element one too far ahead in textedits

Contributor

topisani commented Nov 19, 2017

std::sting -> std::string typo fixit does replaces trailing text content/space, it should insert the new string instead of replacing, likely a bug in FixIt generation

In emacs i subtract 1 from the end of all textedits, since emacs uses an inclusive range, and cquery (i assume) uses an exclusive range, since otherwise replacing 0 characters, ie. just inserting wouldnt be possible.

Edit:
Turns out thats actually not the right thing to do - think the cquery server allways points to an element one too far ahead in textedits

@sztomi

This comment has been minimized.

Show comment
Hide comment
@sztomi

sztomi Nov 26, 2017

Hey,

I'd like to help out with these, can I get a little more context for the following items?

  • make sure we don't load cached file if real file is not present
  • Very make_unique contributes constructor calls
    (^ I don't understand this one, maybe there is a typo in it?)
  • extern const char not included in outline? see ash_switches.h
  • show warning for compile_commands.json, autogen if possible; maybe git rev-parse origin/master ; or git rev-parse HEAD

BTW, thank you for making the only C++ extension for vscode that actually works.

sztomi commented Nov 26, 2017

Hey,

I'd like to help out with these, can I get a little more context for the following items?

  • make sure we don't load cached file if real file is not present
  • Very make_unique contributes constructor calls
    (^ I don't understand this one, maybe there is a typo in it?)
  • extern const char not included in outline? see ash_switches.h
  • show warning for compile_commands.json, autogen if possible; maybe git rev-parse origin/master ; or git rev-parse HEAD

BTW, thank you for making the only C++ extension for vscode that actually works.

@jacobdufault

This comment has been minimized.

Show comment
Hide comment
@jacobdufault

jacobdufault Nov 26, 2017

Member

make sure we don't load cached file if real file is not present

a.cc is indexed (so cache/a.cc and cache/a.cc.json exist), but a.cc was deleted but is still present in compile_commands.json. Make sure that if a.cc is deleted (but cache/a.cc and cache/a.cc.json still exist) we do not load cache/a.cc.json into querydb.

Very make_unique contributes constructor calls

Basically, std::make_unique<T>(a, b) (or make_shared, etc) should add a constructor call to T::T(a, b). See this test. I believe this works but it seemed to be a bit buggy when using it on Chrome.

extern const char not included in outline? see ash_switches.h

Outline in this file does not work correctly.

show warning for compile_commands.json, autogen if possible; maybe git rev-parse origin/master ; or git rev-parse HEAD

If the user has updated their source tree we should detect that and automatically regenerate compile_commands.json for them (especially if using ninja), such that using cquery does not require any additional sync/build steps.

BTW, thank you for making the only C++ extension for vscode that actually works.

Thanks!

Member

jacobdufault commented Nov 26, 2017

make sure we don't load cached file if real file is not present

a.cc is indexed (so cache/a.cc and cache/a.cc.json exist), but a.cc was deleted but is still present in compile_commands.json. Make sure that if a.cc is deleted (but cache/a.cc and cache/a.cc.json still exist) we do not load cache/a.cc.json into querydb.

Very make_unique contributes constructor calls

Basically, std::make_unique<T>(a, b) (or make_shared, etc) should add a constructor call to T::T(a, b). See this test. I believe this works but it seemed to be a bit buggy when using it on Chrome.

extern const char not included in outline? see ash_switches.h

Outline in this file does not work correctly.

show warning for compile_commands.json, autogen if possible; maybe git rev-parse origin/master ; or git rev-parse HEAD

If the user has updated their source tree we should detect that and automatically regenerate compile_commands.json for them (especially if using ninja), such that using cquery does not require any additional sync/build steps.

BTW, thank you for making the only C++ extension for vscode that actually works.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment