Best practices with extensions arguments?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

Best practices with extensions arguments?

Jeremie Bresson
I have created a simple extension that makes an "edit on GitHub" link.
You can see the documentation on this page:
http://jmini.github.io/asciidoctorj-gh-edit/

I am asking me, what the "best practice" for the arguments in the squared brackets is.

For the moment I support two optional parameters. They are not named.
gh-edit:jmini/myrepo[]
gh-edit:jmini/myrepo[my_branch]
gh-edit:jmini/myrepo[master, custom link text]

Should I also support quotes (single, double?)
gh-edit:jmini/myrepo["my_branch"]
gh-edit:jmini/myrepo['my_branch']

Should I also support named parameters:
gh-edit:jmini/myrepo[branch="my_branch"]
In order to be able to do something like that:
gh-edit:jmini/myrepo[link-text="my custom link"]

Are those constructs even supported/recommended by Asciidoctor?
Reply | Threaded
Open this post in threaded view
|

Re: Best practices with extensions arguments?

Robert.Panzer
Hi Jeremie,

supporting both positional and named parameters should be possible.

To get the positional attributes you have to set the content model to attributes:
        Map<String, Object> options = new HashMap<String, Object>();
        options.put("content_model", ":attributes");
        MyMacro inlineMacroProcessor = new MyMacro("gh-edit", options);

In the process method you should be able to get parameter 1 just by calling attributes.get(1).

    protected String process(AbstractBlock parent, String target,
            Map<String, Object> attributes) {
        System.out.println("First attribute is " + attributes.get(1));
    ...
    }

You will also see named attributes under the name in the attributes map.

    protected String process(AbstractBlock parent, String target,
            Map<String, Object> attributes) {
        System.out.println("First attribute is " + attributes.get("branch"));
    ...
    }

If you want the first unnamed attribute to automatically be "branch" and not care in your extension if it is defined via a positional attribute or a named attribute, you have to define the positional attributes during the extension registration:

        Map<String, Object> options = new HashMap<String, Object>();
        options.put("content_model", ":attributes");
        options.put("pos_attrs", new ArrayList(Arrays.asList(":branch")));
        MyMacro inlineMacroProcessor = new MyMacro("gh-edit", options);

(Note the leading colon at the beginning of the attribute name!)

Hope it works for you!

Cheers
Robert
Reply | Threaded
Open this post in threaded view
|

Re: Best practices with extensions arguments?

Robert.Panzer
That said this becomes a lot easier with AsciidoctorJ 1.6.0 :-) as you can then simply define positional attributes via annotations on the Extension:

https://github.com/asciidoctor/asciidoctorj/blob/asciidoctorj-1.6.0/asciidoctorj-core/src/test/groovy/org/asciidoctor/extension/AnnotatedLongInlineMacroProcessor.groovy
Reply | Threaded
Open this post in threaded view
|

Re: Best practices with extensions arguments?

Jeremie Bresson
Those are great inputs!

My current handling is just wrong, I use: attributes.get("text").
I will have to fix it (#3)

----

1.6.0 looks promising... The first step for me to be able to use it would be to create a version of the asciidoctor-maven-plugin that is compatible with this version (related post).
Reply | Threaded
Open this post in threaded view
|

Re: Best practices with extensions arguments?

sclassen
In reply to this post by Robert.Panzer
Thanks Robert

I was wondering why in a block processor I get all the named attributes in the attributes parameter as key value pairs where as for the block macro i get n entry 'text' where the entire content of the [] is placed as a String.

You post pointed me in the direction that this is dependent on how the extension is registered.
Is there some documentation on which options are supported and what they are meant for.

Cheers
Stephan
Reply | Threaded
Open this post in threaded view
|

Re: Best practices with extensions arguments?

mojavelinux
Administrator
In reply to this post by Jeremie Bresson
Great info indeed, Robert!

In general, positional arguments should be an added convenience. You should always allow attributes to be specified long-hand by name. I believe that the configuration Robert pointed to you allows you to map positional attributes to names.

> Jeremie wrote:
>
> Should I also support quotes (single, double?) 
> gh-edit:jmini/myrepo["my_branch"]
> gh-edit:jmini/myrepo['my_branch']

You should not have to worry about single and double quotes. That's something that is handled for you when you parse attributes (by setting the content_model to :attributes).

> Stephan wrote:
>
>  I get all the named attributes in the attributes parameter as key value pairs where as for the block macro i get n entry 'text' where the entire content of the [] is placed as a String.

This is just a matter of different defaults. Since a macro may be used to enclose either content or attributes within the square brackets, it's possible to specify to the parser which one you want. That's where the following option comes in:

options.put("content_model", ":attributes");

The default for a macro (block or inline) is to not parse the value inside the square brackets. Blocks (aka delimited blocks), by distinction, don't allow this level of control.

A good example of this is the button macro. The value inside the square brackets is the content, not an attribute list.

button:[Save, then self-destruct]

When you design the macro, you kind of have to decide whether the value in the square brackets is content or an attribute list.

-Dan

--
Dan Allen | @mojavelinux | http://google.com/profiles/dan.j.allen
Reply | Threaded
Open this post in threaded view
|

Re: Best practices with extensions arguments?

mojavelinux
Administrator
In reply to this post by Jeremie Bresson
As for the macro in question, I might use the file name in the "target" position instead of the repo path. For example:

gh-edit:filename[repo=jmini/myrepo, branch=master]

You could then allow the repo and branch to be specified globally (when it's the same throughout), reducing that to:

gh-edit:filename[]

The "filename" would be the path in the repository to that file.

Try to embrace one of the following forms:

name:<target>[<attrlist>]

name:<main content>[<secondary content>]

name:[<content>]

-Dan

On Sat, Feb 27, 2016 at 7:43 PM, Dan Allen <[hidden email]> wrote:
Great info indeed, Robert!

In general, positional arguments should be an added convenience. You should always allow attributes to be specified long-hand by name. I believe that the configuration Robert pointed to you allows you to map positional attributes to names.

> Jeremie wrote:
>
> Should I also support quotes (single, double?) 
> gh-edit:jmini/myrepo["my_branch"]
> gh-edit:jmini/myrepo['my_branch']

You should not have to worry about single and double quotes. That's something that is handled for you when you parse attributes (by setting the content_model to :attributes).

> Stephan wrote:
>
>  I get all the named attributes in the attributes parameter as key value pairs where as for the block macro i get n entry 'text' where the entire content of the [] is placed as a String.

This is just a matter of different defaults. Since a macro may be used to enclose either content or attributes within the square brackets, it's possible to specify to the parser which one you want. That's where the following option comes in:

options.put("content_model", ":attributes");

The default for a macro (block or inline) is to not parse the value inside the square brackets. Blocks (aka delimited blocks), by distinction, don't allow this level of control.

A good example of this is the button macro. The value inside the square brackets is the content, not an attribute list.

button:[Save, then self-destruct]

When you design the macro, you kind of have to decide whether the value in the square brackets is content or an attribute list.

-Dan

--
Dan Allen | @mojavelinux | http://google.com/profiles/dan.j.allen



--
Dan Allen | @mojavelinux | http://google.com/profiles/dan.j.allen
Reply | Threaded
Open this post in threaded view
|

Re: Best practices with extensions arguments?

mojavelinux
Administrator
In reply to this post by Jeremie Bresson
The way you decide what goes in <target> is to think about what information is required for it to have meaning. That's the target. It's usually a file name (like in the image macro) or an id (like in the video macro).

-Dan

On Sat, Feb 27, 2016 at 7:45 PM, Dan Allen <[hidden email]> wrote:
As for the macro in question, I might use the file name in the "target" position instead of the repo path. For example:

gh-edit:filename[repo=jmini/myrepo, branch=master]

You could then allow the repo and branch to be specified globally (when it's the same throughout), reducing that to:

gh-edit:filename[]

The "filename" would be the path in the repository to that file.

Try to embrace one of the following forms:

name:<target>[<attrlist>]

name:<main content>[<secondary content>]

name:[<content>]

-Dan

On Sat, Feb 27, 2016 at 7:43 PM, Dan Allen <[hidden email]> wrote:
Great info indeed, Robert!

In general, positional arguments should be an added convenience. You should always allow attributes to be specified long-hand by name. I believe that the configuration Robert pointed to you allows you to map positional attributes to names.

> Jeremie wrote:
>
> Should I also support quotes (single, double?) 
> gh-edit:jmini/myrepo["my_branch"]
> gh-edit:jmini/myrepo['my_branch']

You should not have to worry about single and double quotes. That's something that is handled for you when you parse attributes (by setting the content_model to :attributes).

> Stephan wrote:
>
>  I get all the named attributes in the attributes parameter as key value pairs where as for the block macro i get n entry 'text' where the entire content of the [] is placed as a String.

This is just a matter of different defaults. Since a macro may be used to enclose either content or attributes within the square brackets, it's possible to specify to the parser which one you want. That's where the following option comes in:

options.put("content_model", ":attributes");

The default for a macro (block or inline) is to not parse the value inside the square brackets. Blocks (aka delimited blocks), by distinction, don't allow this level of control.

A good example of this is the button macro. The value inside the square brackets is the content, not an attribute list.

button:[Save, then self-destruct]

When you design the macro, you kind of have to decide whether the value in the square brackets is content or an attribute list.

-Dan

--
Dan Allen | @mojavelinux | http://google.com/profiles/dan.j.allen



--
Dan Allen | @mojavelinux | http://google.com/profiles/dan.j.allen



--
Dan Allen | @mojavelinux | http://google.com/profiles/dan.j.allen
Reply | Threaded
Open this post in threaded view
|

Re: Best practices with extensions arguments?

Jeremie Bresson
In reply to this post by mojavelinux
mojavelinux wrote
As for the macro in question, I might use the file name in the "target"
position instead of the repo path. For example:

gh-edit:filename[repo=jmini/myrepo, branch=master]

You could then allow the repo and branch to be specified globally (when
it's the same throughout), reducing that to:

gh-edit:filename[]
Thank you for this feedback.

I had chosen to put the repository name in the target, because I thought this was the only required information. Your argument that it can be specified globally is good. I might want to move this information also to the argument.

In my first version, the filename cannot be specified. I use the docfile value in the document. This is an important feature for me.

I agree with your idea, some of the users might want to specify a filename:
gh-edit:asciidoctor/asciidoctor.org[filename='docs/user-manual.adoc', link-text='edit the Asciidoctor User Manual']

If I decide to move the repository to an argument, can the target be empty?
gh-edit:[repository='asciidoctor/asciidoctor.org', filename='docs/user-manual.adoc', link-text='edit the Asciidoctor User Manual']

Or is it also a bad practice?
Reply | Threaded
Open this post in threaded view
|

Re: Best practices with extensions arguments?

mojavelinux
Administrator
Jérémie wrote: 
I had chosen to put the repository name in the target, because I thought this was the only required information.
In my first version, the filename cannot be specified. I use the docfile value in the document. This is an important feature for me. 

Ah, that makes sense now. In that case, +1
 

I agree with your idea, some of the users might want to specify a filename:
gh-edit:asciidoctor/asciidoctor.org[filename='docs/user-manual.adoc', link-text='edit the Asciidoctor User Manual']
Perhaps in the case when it cannot be implied, for whatever reason. Maybe in the case there isn't a 1-to-1 mapping.

If I decide to move the repository to an argument, can the target be empty?

Absolutely. This is known as the short format. One example of this is the button macro (e.g., button:[Save]). Definitely an accepted practice.

For an example of the short inline macro, see https://github.com/asciidoctor/asciidoctor-extensions-lab/blob/master/lib/pick-inline-macro.rb#L16-L18. I think the referenced bug is resolved in 1.5.4, so you can use the commented line.

-Dan

--
Dan Allen | @mojavelinux | http://google.com/profiles/dan.j.allen
Reply | Threaded
Open this post in threaded view
|

Re: Best practices with extensions arguments?

Jeremie Bresson
Follow up:
Using your inputs, I have released a new version of my small plugin.

You can now do:
[source, asciidoctor]
----
:repository: jmini/jmini.github.io
:branch: develop

Do you want to improve this documentation? Please gh:edit[].
----

or:
[source, asciidoctor]
----
See gh:view[repository='asciidoctor/asciidoctor.org', path="news/debuter-avec-asciidoctor.adoc", link-text='this article in french'] on GitHub.
----

More detail on this page: gh:edit[] Documentation.

Or in this blog post: Documentation: do not forget the edit link
Reply | Threaded
Open this post in threaded view
|

Re: Best practices with extensions arguments?

mojavelinux
Administrator
Jeremie,

These are nice improvements!

I'd like to make one additional suggestion that I think will bring the macro more inline with the pattern in AsciiDoc, take it or leave it :)

I think the mode, edit or view, should be an attribute and the macro should be named "gh-link" (or perhaps you can keep "gh" for short).

In the case when the link is to the current file, it would look like:

gh:[mode=edit]
gh-link:[mode=edit]

or perhaps

gh:self[mode=edit]
gh-link:self[mode=edit]

(I really like having the keyword "self" to make it clear that the link is to the current file).

In the case when the link is to another path, it would look like:

gh:news/debuter-avec-asciidoctor.adoc[this article in french, repository=asciidoctor/asciidoctor.org, mode=view]

I think it's important that the target slot (in the pattern <name>:<target>[<attrlist>]) is reserved for the target. In this case, the path is clearly the target.

You can drop the need for the explicit link-text attribute by making the link-text the first positional attribute. Asciidoctor supports this.

These types of iterative changes are what really make the syntax achieve a very natural and intuitive feel. I hope you're willing to keep iterating.

Cheers!

-Dan

On Thu, Apr 21, 2016 at 11:32 PM, Jeremie Bresson [via Asciidoctor :: Discussion] <[hidden email]> wrote:
Follow up:
Using your inputs, I have released a new version of my small plugin.

You can now do:
[source, asciidoctor]
----
:repository: jmini/jmini.github.io
:branch: develop

Do you want to improve this documentation? Please gh:edit[].
----

or:
[source, asciidoctor]
----
See gh:view[repository='asciidoctor/asciidoctor.org', path="news/debuter-avec-asciidoctor.adoc", link-text='this article in french'] on GitHub.
----

More detail on this page: gh:edit[] Documentation.

Or in this blog post: Documentation: do not forget the edit link


If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/Best-practices-with-extensions-arguments-tp4404p4642.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML



--
Dan Allen | @mojavelinux | http://google.com/profiles/dan.j.allen
Reply | Threaded
Open this post in threaded view
|

Re: Best practices with extensions arguments?

mojavelinux
Administrator
In reply to this post by Jeremie Bresson

On Fri, Apr 22, 2016 at 12:04 AM, Dan Allen <[hidden email]> wrote:
I'd like to make one additional suggestion

okay, so it was more than one suggestion ;)


--
Dan Allen | @mojavelinux | http://google.com/profiles/dan.j.allen
Reply | Threaded
Open this post in threaded view
|

Re: Best practices with extensions arguments?

Jeremie Bresson
In reply to this post by mojavelinux
@Dan:
Thank you for this feedback.

I think I will change the API and publish a new version. I have noticed that this approach can be used with other Git Manager like GitLab.

I think the macro will become: "git-link:<target>[<attributes>]"

I like the "self" value for the target.

As with the current version, I want to provide the possibility to define some stuff at document level. I think that the attributes should be prefixed also with "git-".
That would be:

You can git-link:some/folder/myfile.adoc[repository="myteam/myproject", mode="view", link-text="view this file", server="https://git.ourcompany.com/"] directly on GitLab.

Or at document level:

:git-server: https://github.com/
:git-repository: asciidoctor/asciidoctor.org
:git-branch: master
:git-mode: edit

You can git-link:news/debuter-avec-asciidoctor.adoc[link-text="edit this article in french"] directly on GitHub.

Maybe the default mode should also be "view" instead of "edit".