Re: [dpdk-dev] [PATCH] doc/contributing/documentation: add info about including code

From: Walsh, Conor <hidden>
Date: 2021-05-06 13:50:58

<snip>

quoted

quoted
quoted
quoted
quoted
quoted
quoted
21/04/2021 11:11, Conor Walsh:
quoted
+  The following will include a snippet from the skeleton sample

app::

quoted

+
+      .. literalinclude:: ../../../examples/skeleton/basicfwd.c
+        :language: c
+        :start-after: Display the port MAC address.
+        :end-before: Enable RX in promiscuous mode for the

Ethernet device.

quoted

+        :dedent: 1

I would prefer indenting the options with 3 spaces
to make them aligned with literalinclude.

I will update this in v2.

quoted

quoted
quoted
quoted
quoted
quoted
quoted
[...]
quoted
+* ``start-after`` and ``end-before`` can use any text within a given

file,

quoted

quoted
quoted
quoted
quoted
quoted
quoted
quoted
+  however it may be difficult to find unique text within your code

to mark the

quoted

quoted
quoted
quoted
quoted
quoted
quoted
quoted
+  start and end of your snippets. In these cases, it is

recommended to include

quoted

quoted
quoted
quoted
quoted
quoted
quoted
quoted
+  explicit tags in your code to denote these locations for

documentation purposes.

quoted

+
+  This can be done as follows:
+
+  .. code-block:: c
+
+    /* #guide_doc: Example feature being documented. */
+    ...
+    /* #guide_doc: End of example feature being documented. */

I think we can standardize this usage in a beautiful syntax.
My proposal, using the scissor sign:

      /* Foo bar >8 */
      foo(bar);
      /* 8< End of foo bar */

I like the scissors syntax, I think its lightweight, clear and will help provide a unique string to search for.
I would switch the direction of the scissors so that the snippet is between the cuts to make it a bit clearer.
/* Foo bar 8< */
foo(bar);
/* >8 End of foo bar */

quoted

      .. literalinclude:: foobar.c
         :language: C
         :start-after: Foo bar >8
         :end-before: 8< End of foo bar

Another idea:

      /*~ Foo bar */
      foo(bar);
      /*~ End of foo bar */

      .. literalinclude:: foobar.c
         :language: C
         :start-after: ~ Foo bar
         :end-before: ~ End of foo bar

Maybe we don't need any markup for the start line and keep it

natural:

quoted

      /* Foo bar */
      foo(bar);
      /* end: Foo bar */

      .. literalinclude:: foobar.c
         :language: C
         :start-after: Foo bar
         :end-before: end: Foo bar

I think it definitely needs some syntax so that people realise it isn’t just a normal comment.

quoted

quoted
quoted
quoted
quoted
quoted
Not having markup will 1) risk people accidentally "fixing" or

otherwise

quoted

quoted
quoted
quoted
quoted
quoted
modifying comments, and 2) has bigger potential for collisions

elsewhere

quoted

in the comments. While these aren't big risks, IMO it should be
explicitly obvious that a comment is not just a comment but a marker

docs.

quoted

Having named tags like in the original proposal is the most explicit
version of the above, which is why i favor it, but i think it's OK to
have a lighter-weight syntax (e.g. with scissors for example),

however i

quoted

don't think it's a good idea to leave things implicit like your last
suggestion.

I think the first comment is not only for code extraction,
but also for code reader, that's why I think it's good to keep it natural.

+1 to Anatoly's comment, to make it obvious to the reader of the code

that the

quoted

comment is used for documentation purposes and use explicit syntax

for it.

quoted

So you assume the comment is only for doc extraction?

I think the comments added as part of this should be meaningful comments that include the scissors syntax.
If there is already a multiline comment present then the last line should be the name of the feature or something short with the scissors syntax.

quoted

I think it can be a real comment, otherwise we'll need to have
2 lines: 1 for doc extraction, 1 for code comment.

I see your point, for the cases that there is already a comment before (or
after) the code, will it be too bad to have multiple lines, something like:

/* Actual comment
  * More details
  *
  * explicit marker */

I can add an example that cover this case in v2.
/* Comments
  * More comments
  *
  * Foo Bar 8< */
foo(bar);
/* >8 End of foo bar */

quoted


I think explicit marker has the advantage of:
- Whoever updating the comment will know that it is a marker for the
documentation and be careful on update
- Whoever updating the code between the markers, know that it may be

required to

quoted

re-visit the relevant documentation and update it because of code change
- Whoever reading the code will know that part is in a documentation, and

may be

quoted

interested to go and check the relevant documentation
- Whoever reading the code, and not very familiar with DPDK convention,

still

quoted

can understand what that comment is for and benefit from above

I understand these points.
But I'm afraid the proposed syntax #guide_doc:
is not so obvious for everybody.

I'm sure there can be a better syntax.

I'm not particularly attached to either syntax, as long as it's clearly
documented and explicitly visible in the code (as opposed to something
that kinda-sorta-but-doesn't-really-look-like-a-marker type syntax).

So, we'd have to give up some "natural-ness", as *that's the point* of
having a syntax. And i'm certainly OK with comment style like this:

/*
  * some meaningful comment.
  * :doc_marker:
  */
...
/* :end doc_marker: */

Or something to that effect. What i'm *not* in favor of is these markers
looking like part of a comment, rather than markers for doxygen.

I agree I think the scissors syntax would be a good mix between having a marker and keeping it as a natural comment while also showing that it isn’t a standard comment.

Thanks for everyone's feedback and input.
I will send a v2 based on these comments.

Thanks,
Conor.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help