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

don't error on ModuleCompletions for module where @doc isn't defined #193

Merged
merged 1 commit into from
Sep 30, 2019

Conversation

aviatesk
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Sep 28, 2019

Codecov Report

Merging #193 into master will not change coverage.
The diff coverage is 90%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #193   +/-   ##
=======================================
  Coverage   28.53%   28.53%           
=======================================
  Files          35       35           
  Lines        1514     1514           
=======================================
  Hits          432      432           
  Misses       1082     1082
Impacted Files Coverage Δ
src/utils.jl 92.64% <100%> (ø) ⬆️
src/eval.jl 18.47% <100%> (ø) ⬆️
src/completions.jl 76.57% <87.5%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e04d536...697b0e6. Read the comment docs.

@aviatesk aviatesk merged commit fa3ed71 into master Sep 30, 2019
@aviatesk aviatesk deleted the avi/completionerror branch September 30, 2019 07:17
@pfitzseb
Copy link
Member

Isn't this the "wrong" fix and we should be using something like Main.@doc here?

@aviatesk
Copy link
Member Author

Sorry for hasty merge.

Is there any case that there are docs where @doc is not defined ?

If so, maybe changing this line to something like

word = "$mod.$word"
Core.eval(Main, :(@doc($(Symbol(word)))))

will help ?

@aviatesk
Copy link
Member Author

Is there any case that there are docs where @doc is not defined ?

Ah yeah, we can attach docs to objs after they gets defined.

I will make a PR to fix this

@pfitzseb
Copy link
Member

No, not all modules are loaded in Main (e.g. package dependencies -- even though Atom depends on CodeTools, you can't access Main.CodeTools).

Is there any case that there are docs where @doc is not defined ?

Pretty sure that e.g. a baremodule doesn't have @doc defined, but docstrings work just fine.

@aviatesk
Copy link
Member Author

No, not all modules are loaded in Main (e.g. package dependencies -- even though Atom depends on CodeTools, you can't access Main.CodeTools).

Yeah, definitely.

So might be calling @doc word from a module where completions are invoked is a possible solution, right ? (of course, only if @doc is defined in the module)

@aviatesk
Copy link
Member Author

aviatesk commented Sep 30, 2019

so something like

completionsummary(mod, c::REPLCompletions.ModuleCompletion) = begin
  m, word = c.parent, c.mod
  cangetdocs(m, word) || return ""
  docs = if isdefined(m, Symbol("@doc"))
    getdocs(m, word)
  else
    word = string(m) * "." * word
    getdocs(mod, word)
  end
  description(docs)
end

with the previous cangetdocs

aviatesk added a commit that referenced this pull request Sep 30, 2019
aviatesk added a commit that referenced this pull request Sep 30, 2019
aviatesk added a commit that referenced this pull request Oct 7, 2019
aviatesk added a commit that referenced this pull request Oct 7, 2019
pfitzseb added a commit that referenced this pull request Oct 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants