Re: [PATCH] Documentation: implement linkgit macro for Asciidoctor

[Junio C Hamano]

"brian m. carlson"  writes:

> On Thu, Jan 26, 2017 at 07:18:41PM +, Eric Wong wrote:
>> > Eric Wong  writes:
>> Junio C Hamano  wrote:
>> > +  "\n"
>> > +"#{target}"
>> > +"#{attrs[1]}\n"
>> > +  "\n"
>> >  end
>> 
>> You need the '\' at the end of those strings, it's not like C
>> since Ruby doesn't require semi-colons to terminate lines.
>> In other words, that should be:
>> 
>>   "\n" \
>> "#{target}" \
>> "#{attrs[1]}\n" \
>>   "\n"
>> 
>
> This change is fine with me.

OK, I just squashed the final one in.  Will merge to 'next' shortly.

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 11f76506b6..b21e5808b1 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -179,7 +179,8 @@ ASCIIDOC = asciidoctor
 ASCIIDOC_CONF =
 ASCIIDOC_HTML = xhtml5
 ASCIIDOC_DOCBOOK = docbook45
-ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions -alitdd='&\#x2d;&\#x2d;'
+ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
+ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
 DBLATEX_COMMON =
 endif
 
diff --git a/Documentation/asciidoctor-extensions.rb 
b/Documentation/asciidoctor-extensions.rb
index 09f7088eea..ec83b4959e 100644
--- a/Documentation/asciidoctor-extensions.rb
+++ b/Documentation/asciidoctor-extensions.rb
@@ -13,10 +13,10 @@ module Git
   prefix = parent.document.attr('git-relative-html-prefix')
   %(#{target}(#{attrs[1]})\n)
 elsif parent.document.basebackend? 'docbook'
-  %(
-#{target}#{attrs[1]}
-
-)
+  "\n" \
+"#{target}" \
+"#{attrs[1]}\n" \
+  "\n"
 end
   end
 end

Re: [PATCH] Documentation: implement linkgit macro for Asciidoctor

[brian m. carlson]

On Thu, Jan 26, 2017 at 07:18:41PM +, Eric Wong wrote:
> > Eric Wong  writes:
> Junio C Hamano  wrote:
> > +  "\n"
> > +"#{target}"
> > +"#{attrs[1]}\n"
> > +  "\n"
> >  end
> 
> You need the '\' at the end of those strings, it's not like C
> since Ruby doesn't require semi-colons to terminate lines.
> In other words, that should be:
> 
>   "\n" \
> "#{target}" \
> "#{attrs[1]}\n" \
>   "\n"
> 

This change is fine with me.

For the record, I don't have a strong opinion one way or the other.
Since this code is related to Asciidoctor and Git has no existing Ruby
style standards, I picked the Asciidoctor house style, which uses
multi-line %().  We could pick [0] as an option, or just argue it out
when someone cares, like here.

[0] https://github.com/bbatsov/ruby-style-guide
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH] Documentation: implement linkgit macro for Asciidoctor

[Eric Wong]

> Eric Wong  writes:
> > You can use '\' to continue long lines with any Ruby version:
> >
> > "" \
> >   "#{target}" \
> >   "#{attrs[1]}" \
> > ""

Junio C Hamano  wrote:
> +  "\n"
> +"#{target}"
> +"#{attrs[1]}\n"
> +  "\n"
>  end

You need the '\' at the end of those strings, it's not like C
since Ruby doesn't require semi-colons to terminate lines.
In other words, that should be:

  "\n" \
"#{target}" \
"#{attrs[1]}\n" \
  "\n"

Re: [PATCH] Documentation: implement linkgit macro for Asciidoctor

[Johannes Schindelin]

Hi Brian,

On Thu, 26 Jan 2017, brian m. carlson wrote:

> AsciiDoc uses a configuration file to implement macros like linkgit,
> while Asciidoctor uses Ruby extensions.  Implement a Ruby extension that
> implements the linkgit macro for Asciidoctor in the same way that
> asciidoc.conf does for AsciiDoc.  Adjust the Makefile to use it by
> default.

I like it.

Thank you,
Johannes

Re: [PATCH] Documentation: implement linkgit macro for Asciidoctor

[Eric Wong]

Jeff King  wrote:
> On Thu, Jan 26, 2017 at 12:13:44AM +, brian m. carlson wrote:
> > +
> > +  def process(parent, target, attrs)
> > +if parent.document.basebackend? 'html'
> > +  prefix = parent.document.attr('git-relative-html-prefix')
> > +  %(#{target}(#{attrs[1]})\n)
> > +elsif parent.document.basebackend? 'docbook'
> > +  %(
> > +#{target}#{attrs[1]}
> > +
> > +)



> The multi-line string is kind of ugly because of the indentation.
> Apparently Ruby has here-docs that will eat leading whitespace, but the
> syntax was not introduce until Ruby 2.3, which is probably more recent
> than we should count on.

You can use '\' to continue long lines with any Ruby version:

"" \
  "#{target}" \
  "#{attrs[1]}" \
""

The above happens during the parse phase, so there's no garbage
or method call overhead compared to the more-frequently seen '+'
or '<<' method calls to combine strings.

> I think you could write:
> 
>   %(
> 
> #{target}#{attrs[1]}
> 
> ).gsub(/^\s*/, "")
> 
> I don't know if that's too clever or not.

Ick...

> But either way, I like this better than introducing an extra dependency.

Agreed.

Re: [PATCH] Documentation: implement linkgit macro for Asciidoctor

[Jeff King]

On Thu, Jan 26, 2017 at 12:13:44AM +, brian m. carlson wrote:

> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 19c42eb60..d1b7a6865 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -179,10 +179,7 @@ ASCIIDOC = asciidoctor
>  ASCIIDOC_CONF =
>  ASCIIDOC_HTML = xhtml5
>  ASCIIDOC_DOCBOOK = docbook45
> -ifdef ASCIIDOCTOR_EXTENSIONS_LAB
> -ASCIIDOC_EXTRA = -I$(ASCIIDOCTOR_EXTENSIONS_LAB) -rasciidoctor/extensions 
> -rman-inline-macro
> -endif
> -ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
> +ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions -alitdd='&\#x2d;&\#x2d;'

Might be more readable to just leave the litdd part on its own line.

> diff --git a/Documentation/asciidoctor-extensions.rb 
> b/Documentation/asciidoctor-extensions.rb
> new file mode 100644
> index 0..09f7088ee
> --- /dev/null
> +++ b/Documentation/asciidoctor-extensions.rb
> @@ -0,0 +1,28 @@
> +require 'asciidoctor'
> +require 'asciidoctor/extensions'
> +
> +module Git
> +  module Documentation
> +class LinkGitProcessor < Asciidoctor::Extensions::InlineMacroProcessor
> +  use_dsl
> +
> +  named :chrome
> +
> +  def process(parent, target, attrs)
> +if parent.document.basebackend? 'html'
> +  prefix = parent.document.attr('git-relative-html-prefix')
> +  %(#{target}(#{attrs[1]})\n)
> +elsif parent.document.basebackend? 'docbook'
> +  %(
> +#{target}#{attrs[1]}
> +
> +)
> +end
> +  end
> +end
> +  end
> +end

I think this looks reasonable. There's some boilerplate, but even as
somebody not familiar with asciidoctor, it's all quite obvious.

The multi-line string is kind of ugly because of the indentation.
Apparently Ruby has here-docs that will eat leading whitespace, but the
syntax was not introduce until Ruby 2.3, which is probably more recent
than we should count on.

I think you could write:

  %(

#{target}#{attrs[1]}

  ).gsub(/^\s*/, "")

I don't know if that's too clever or not.

But either way, I like this better than introducing an extra dependency.

-Peff

[PATCH] Documentation: implement linkgit macro for Asciidoctor

[brian m. carlson]

AsciiDoc uses a configuration file to implement macros like linkgit,
while Asciidoctor uses Ruby extensions.  Implement a Ruby extension that
implements the linkgit macro for Asciidoctor in the same way that
asciidoc.conf does for AsciiDoc.  Adjust the Makefile to use it by
default.

Signed-off-by: brian m. carlson 
---
 Documentation/Makefile  |  5 +
 Documentation/asciidoctor-extensions.rb | 28 
 2 files changed, 29 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/asciidoctor-extensions.rb

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 19c42eb60..d1b7a6865 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -179,10 +179,7 @@ ASCIIDOC = asciidoctor
 ASCIIDOC_CONF =
 ASCIIDOC_HTML = xhtml5
 ASCIIDOC_DOCBOOK = docbook45
-ifdef ASCIIDOCTOR_EXTENSIONS_LAB
-ASCIIDOC_EXTRA = -I$(ASCIIDOCTOR_EXTENSIONS_LAB) -rasciidoctor/extensions 
-rman-inline-macro
-endif
-ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
+ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions -alitdd='&\#x2d;&\#x2d;'
 DBLATEX_COMMON =
 endif
 
diff --git a/Documentation/asciidoctor-extensions.rb 
b/Documentation/asciidoctor-extensions.rb
new file mode 100644
index 0..09f7088ee
--- /dev/null
+++ b/Documentation/asciidoctor-extensions.rb
@@ -0,0 +1,28 @@
+require 'asciidoctor'
+require 'asciidoctor/extensions'
+
+module Git
+  module Documentation
+class LinkGitProcessor < Asciidoctor::Extensions::InlineMacroProcessor
+  use_dsl
+
+  named :chrome
+
+  def process(parent, target, attrs)
+if parent.document.basebackend? 'html'
+  prefix = parent.document.attr('git-relative-html-prefix')
+  %(#{target}(#{attrs[1]})\n)
+elsif parent.document.basebackend? 'docbook'
+  %(
+#{target}#{attrs[1]}
+
+)
+end
+  end
+end
+  end
+end
+
+Asciidoctor::Extensions.register do
+  inline_macro Git::Documentation::LinkGitProcessor, :linkgit
+end
-- 
2.11.0