-
Notifications
You must be signed in to change notification settings - Fork 11
Implement an initial API #2
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
Conversation
src/CodeTracking.jl
Outdated
| file, line = fileline(lin) | ||
| end | ||
| if !isabspath(file) | ||
| # This is a Base method |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or Core ?
d610a67 to
8d00f9f
Compare
definition(method, String) and add internal data
Codecov Report
@@ Coverage Diff @@
## master #2 +/- ##
=====================================
Coverage ? 0%
=====================================
Files ? 3
Lines ? 63
Branches ? 0
=====================================
Hits ? 0
Misses ? 63
Partials ? 0
Continue to review full report at Codecov.
|
|
Well, this has grown. I think this might be the complete API in terms of currently-planned functionality. I also have a local Revise branch that interacts successfully with this package. |
b23d66e to
51dfd33
Compare
|
I've pushed a I'm eager to get this registered (3-day delay), so barring concerns about the API I will merge later this morning. |
|
Should have pinged @KristofferC. |
src/CodeTracking.jl
Outdated
| definition(method::Method) = definition(method, Expr) | ||
|
|
||
| """ | ||
| info = pkgfiles(id::PkgId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PkgId itself is AFAIU undocumented and internal. So should the API be based on package name and UUID (which are public) rather than the internal struct PkgId?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or on PackageSpec (which is also exported, iirc)?
| function pkgfiles(name::AbstractString) | ||
| project = Base.active_project() | ||
| uuid = Base.project_deps_get(project, name) | ||
| uuid == false && error("no package ", name, " recognized") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to error here or just return nothing like when it's a real installed package that just hasn't been loaded yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Erroring seems like a good choice to me. And it is at least the conservative choice which can be relaxed later.
Here's the beginning of real forward-movement on this package. The proposed public API is in
CodeTracking.jlitself, the other files are internal. Comments welcome.