MichaelHatherly / Docile.jl

Julia package documentation system.
Other
40 stars 15 forks source link

Proposal: Make $(metaname) part of Docile parseing #102

Closed peter1000 closed 9 years ago

peter1000 commented 9 years ago

come from discussion here

I propose that: $(metaname) should be resolved within Dociles parseing process.

Different than !!new_meta(New Meta Value) this would allow any available metadata to be interpolated into the documentation: similar to julias string interpolation.


Advantages:

BIG question: how are the different available metas resolved: not sure

MichaelHatherly commented 9 years ago

We'd need to require use of an @doc_str macro when using what you propose. $ also clashes with LaTeX syntax which I definitely don't want to lose.

I kind of like the simplicity of only requiring one embedding syntax.

It's true that nesting would be useful sometimes though, perhaps pursuing that first would be best?

BIG question: how are the different available metas resolved: not sure

It's just the Docile.Cache.findmeta function. There's nothing else involved I believe.

peter1000 commented 9 years ago

About which meta is used as replacement one could also an additional simple option: example

for more advanced usage: simple option

peter1000 commented 9 years ago

I will add here my idea od the meta: just give me a second

ProjectMeta ├── Pkg1 Meta │   ├── Module 1 Meta │   │   ├── Object 1 Meta │   │   └── Object 2 Meta │   └── Module 2 Meta │   ├── Object 1 Meta │   └── Object 2 Meta └── Pkg2 Meta ├── Module 1 Meta │   ├── Object 1 Meta │   └── Object 2 Meta └── Module 2 Meta ├── Object 1 Meta └── Object 2 Meta

To each of this items meta should be separate entities and not just a collection of the sub items.

peter1000 commented 9 years ago

I'm not really familar with Docile.Cache.findmeta but I think one might want to like to define which metha is added if there are several metas with author

ProjectJuliaDocumentation author=Mike,peter,Lisa Pkg1 has author=Mike Pkg2.Module1 author=peter Pkg2.Module2 author=Lisa

In Pkg2.Module2.common_function author should refer to all ProjectJuliaDocumentation author

peter1000 commented 9 years ago

. $ also clashes ....

I think anything would be open for usage just thought that $ would seem most natural to julia user.

Cheers

MichaelHatherly commented 9 years ago

$ would seem most natural to julia user

To me "$(...)" would be interpolating a Julian expression into a string such as "$(1 + a)" where a is defined in the current scope. The proposed behaviour doesn't make use of the string's surrounding scope which may be surprising to new users.

peter1000 commented 9 years ago

The proposed behaviour doesn't make use of the string's surrounding scope which may be surprising to new users.

you might be right on that - I believe you will come up with something suitable as in the other cases.

MichaelHatherly commented 9 years ago

You're welcome to pursue this issue, I'm not adverse to the idea of having something like this more easily doable.

On a related note, interestingly we do in fact already have nested !!:

Sandbox.jl:

module Sandbox

"""
!!author(Michael Hatherly)
!!copyright((c) !!author() and contributors.)
"""
Sandbox

end

.docile:

import Docile: Cache, Formats

Formats.metamacro(::Formats.MetaMacro{:author}, body, mod, obj) = isempty(body) ?
    Cache.findmeta(mod, obj, :author) :
    (Cache.getmeta(mod, obj)[:author] = strip(body); "")

function Formats.metamacro(::Formats.MetaMacro{:copyright}, body, mod, obj)
    body = Formats.extractmeta!(body, mod, obj)
    body = Cache.getmeta(mod, obj)[:copyright] = strip(body)
    """
    # Copyright Notice

    $(body)

    """
end

immutable MarkdownFormatter <: Formats.AbstractFormatter end

Formats.parsedocs(::Formats.Format{MarkdownFormatter}, raw, mod, obj) =
    Base.Markdown.parse(raw)

Dict(
    :format => MarkdownFormatter
)

Note the call to Formats.extractmeta! above.

julia> require("Sandbox.jl")

julia> using Docile

julia> Docile.Cache.getparsed(Sandbox)[Sandbox]
INFO: Updating cached package list...
INFO: Initialising cache for 'Sandbox' and related modules...
 Sandbox ✓
INFO: Succcessfully initialised package cache.
     Copyright Notice
    ≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡

  (c) Michael Hatherly and contributors.
peter1000 commented 9 years ago

we do in fact already have nested !!:

Not really I think: as it needs to be implemented for each metaname. So one can not just say it's a general feature which works in any case.

MichaelHatherly commented 9 years ago

Not really I think: as it needs to be implemented for each metaname. So one can not just say it's a general feature which works in any case.

Having to implement the line body = Formats.extractmeta!(body, mod, obj)could be thought of as a feature ;) Means you can disallow it in some cases where it isn't needed. But yeah, I wasn't trying to say it was all already completed.

MichaelHatherly commented 9 years ago

Another possiblilty would be to have an official !!get(...) and !!set(...) that would work something like

metamacro(::META"get", body, mod, obj) = Cache.findmeta(mod, obj, symbol(strip(body)))

function metamacro(::META"set", body, mod, obj)
    var, val = split(body, ':', limit = 2)
    Cache.getmeta(mod, obj)[symbol(var)] = val
    ""
end

So you'd then do

"""
!!set(author:Michael Hatherly)
!!copyright((c) !!get(author) and contributors.)
"""

We could have more complex internal syntax for where to search for the variable as you've suggested above already.

The point is, this is all customisable by the package author without need to change anything in Docile which is quite nice. Maybe the !! syntax needs changing to something more terse? Though I haven't had any bright ideas about what yet.

peter1000 commented 9 years ago

I like the: set / get idea much better than the current implementation and also better than the $(meta) idea of mine :+1:

Maybe the !! syntax needs changing to something more terse?

Would it not work if we just require this:

or have a prefix/postfix to make it more unique as one might write documentation for an other set method or use the escapes.

"""
Sets the absolute path.

Example:

   set(pathname::AbstractString)
"""

The above would be a problem as we have the whole syntax: set(pathname::AbstractString)

Seems the nices solution to me if one wants to refer something just use the escape.

"""
Returns the absolute path.

Example:

   \\set(pathname::AbstractString)
"""
MichaelHatherly commented 9 years ago

I'd like to not treat any metamacro such as !!get or !!set differently to others by giving special syntax. Either we need to find a suitable single Char prefix for all metamacros or just keep !!.

As your \\set(pathname::AbstractString) example shows not requiring a syntax for some leads to things like that. If the author happens to not be aware of the special handling of set it will lead to errors. We should avoid that if possible; explicit rather than implicit.

set within a set should be an error

Seems reasonable to me.

peter1000 commented 9 years ago

I'd like to not treat any metamacro such as !!get or !!set differently to others by giving special syntax.

I thought there would be no others: only 2 set(metaname: ..) and get(metaname) maybe I misunderstand you here.

MichaelHatherly commented 9 years ago

I thought there would be no others

For basic getting and setting of metadata fields, yes !!get and !!set would be the only ones needed. Metamacros can extend to things other than getting and setting variables, such as code examples with automatically generated output:

function metamacro(::META"example", body, mod, obj)
    """
    Example:
    ```jl
    $(strip(body))
```
$(include_string(body))
```
"""

end


``` jl
"""
!!example(
using ASCIIPlots

xs = linspace(-pi, pi)
ys = sin(xs)

lineplot(xs, ys)
)

!!example(
fac(n) = n < 2 ? 1 : fac(n - 1) * n
fac(12)
)
"""
peter1000 commented 9 years ago

In that case I think the double !!are just fine as in normal situations this need not to be escaped.

peter1000 commented 9 years ago

THOUGHT: should parsed !!meta tags be readonly stored.

I tried this:

parsed = Docile.Cache.getparsed(Sandbox, obj)
meta_ = Docile.Cache.getmeta(Sandbox, obj)[:section]
Docile.Cache.getmeta(Sandbox, obj)[:section] = "HELLO"   #THOUGHT this should not be allowed for parsed `!!meta`
parsed = Docile.Cache.getparsed(Sandbox, obj)
println("\nparsed: ", parsed)
meta_ = Docile.Cache.getmeta(Sandbox, obj)[:section]
println("\nmeta_: ", meta_)

OUTPUT:

parsed: example2: One section meta: Lexicon.md/Exported/Methods/1

meta_: HELLO


MichaelHatherly commented 9 years ago

I'd thought about this as well. We could always return a copy from getraw, getmeta, and getparsed.

That's a bit wasteful though, so I'd think we should stick with the "consenting adults" principle and just explain in the documentation for Cache that mutating content in the returned Dicts is likely to cause bad things to happen.

peter1000 commented 9 years ago

Maybe an other option would be to separate the meta extracted from docstrings with just general meta added.

I don't know all what oyu have in mind but from my current plans I think that would be a cleaner solution. goese back to my graph

that I would like to add manually metas to: modules, packages and projects (multi-pkg)

I know I can do that already - but definitly the option to overwrite the obj extracted meta is not good.

maybe store the auto extracted obj metas in a separete dict / whatever and not allowing them to be overwritten.

than for the final obj meta just merge them together with any others.

obj metas:

allobj metas merge(docstr_metas, other_metas)

peter1000 commented 9 years ago

just thought:codesource meta should probably also be overwritten - in fact probably most metas once set should not be overwritten

MichaelHatherly commented 9 years ago

I think we should either have everything readonly or stick with read/write for the moment. Having two separate metadatas seems to be over engineering things to soon.

I'm happy with read/write since metamacros are quite low-level details that won't be touched that often, but if there's a nice clean solution that solves this then I'd be open to changes here.

peter1000 commented 9 years ago

stick with read/write for the moment but as you said beingopen to change if actual issues arise or if no real case is found where metas should be overwritten. :+1:

peter1000 commented 9 years ago

just switch one shortly - I see you added some more code: does this check for the error:

meta within meta?

!!meta(text !!innermeta(text) should raise an error)

comment

set within a set should be an error

MichaelHatherly: Seems reasonable to me.

MichaelHatherly commented 9 years ago

Currently no, only !!longform allows nesting, so internal !! calls for the others just come out as literal text. My thinking is that as we add more and actually make use of them all in the documentation we'll get a better understanding of whether we need additional features for some of them.

peter1000 commented 9 years ago

Lot of nice things you added. also a good idea: !!longform

MichaelHatherly commented 9 years ago

Lot of nice things you added. also a good idea: !!longform

Thanks.

peter1000 commented 9 years ago

QUESTION: I stumbled upon this (took me a while to fined the problem

"\!!set(one_backslash_escape:One backslash is skipped and treated like a normal meta syntax.) \!!get(one_backslash_escape))"
one_backslash_escape = ()

I wrote a test case

@fact Cache.getparsed(MetadataSyntax, :one_backslash_escape) => "One backslash is skipped and treated like a normal meta syntax."

Result

Formats.
     - \!!set.
  Failure   :: (line:-1) :: \!!set. :: got  One backslash is skipped and treated like a normal meta syntax.)
    Cache.getparsed(MetadataSyntax,:one_backslash_escape) => One backslash is skipped and treated like a normal meta syntax.
     - !! set.
Out of 1 total fact:
  Failed:   1
ERROR: LoadError: FactCheck finished with 1 non-successful tests.

It took me a while to figure out what the problem was ;)

RETURN adds one white space in front the bracket at the end.

< One backslash is skipped and treated like a normal meta syntax.)>

this is not acceptable I think - maybe you know how to raise for that an error - or do you want such be parsed?

peter1000 commented 9 years ago

An other thing: I wanted to write a test for undefined metamacro but got stuck - somthing like this

"!!undefined (Undefined MetaMacro.)"
undefined_metamacro = ()

Not sure how to do that in fact - maybe it's simple but I skipped it for now.

MichaelHatherly commented 9 years ago

A single \ does not escape a metamacro, so that parsed output looks correct to me. Am I missing something obvious?

"!!undefined (Undefined MetaMacro.)"

You could write it in a separate module by itself and then do a @fact_throws Cache.getparsed(...) I think.

MichaelHatherly commented 9 years ago

The following works:

module UndefinedMetaMacro

"!!undefined()"
undefined = ()

end
@fact_throws ErrorException Docile.Cache.getparsed(UndefinedMetaMacro, :undefined)
peter1000 commented 9 years ago

A single \ does not escape a metamacro, so that parsed output looks correct to me. Am I missing something obvious?

I had an error at the end - a wrong double bracket - my bad sorry ;)

thanks for the test example I will add it.

MichaelHatherly commented 9 years ago

I had an error at the end

Oh... I didn't scroll sideways far enough.

MichaelHatherly commented 9 years ago

So, with the set of metamacros we have I don't believe we need to parse $(...) in Docile. Closing the issue.