Skip to content

Make route statement available#15

Open
flying-foozy wants to merge 7 commits into
blockdiag:masterfrom
flying-foozy:make-route-statement-available
Open

Make route statement available#15
flying-foozy wants to merge 7 commits into
blockdiag:masterfrom
flying-foozy:make-route-statement-available

Conversation

@flying-foozy

Copy link
Copy Markdown
Contributor

This series makes "route" statement available for nwdiag.
It is assumed that "route" statement was introduced for the diagram as below.
route

Before this commit, route_inline_stmt() cloud not parse inline route
statement like "node1 -> node2", because '->' tokens are discarded by
using op_() instead of op(), even though original make_route()
requires them as "edge_type".

This commit is just one of preparations to make "route" statement
parsable, because there is no code path invoking route_inline_stmt(),
yet.
This commit newly defines route_inline_stmt_list(), in order to accept
multiple inline statements like below::

    route {
        node1 -> node2;
        node2 -> node3;
    }

This commit is just one of preparations to make "route" statement
parsable, because there is no code path invoking
route_inline_stmt_list(), yet.
In order to make parser() correctly parse "route" statement, this
commit applies changes below at once, because these (at least, other
than "ID for route") are tightly coupled.

  + apply skip() on ID value for "route" statement

    ID for "route" might be useless, and have to be removed. But it is
    not scope of this commit.

  + make route_stmt invoke route_inline_stmt_list() instead of
    network_inline_stmt_list(), because this parses not "network"
    statement, but "route" one

  + make original make_route() return not Route object but list of
    Edge objects (and renaming original it to "make_route_edges")

    This also makes route_inline_stmt_list() return a list of lists of
    Edge objects.

  + make route_stmt invoke newly defined make_route() instead of
    constructor of Network

    As described above, route_inline_stmt_list() returns nested Edge
    lists. Therefore, it should be flattened before Route object
    construction. This is reason why Route is directly constructed
    from route_stmt.

After this commit, parser() can correctly parse "route" statement, but
"nwdiag" command itself can not accept any "route" statement, because
DiagramTreeBuilder can not yet build Route elements correctly.
Before this commit, DiagramTreeBuilder could not treat parser.Route
object, because the former misunderstood about the latter: the latter
does not have "nodes" attribute but "edges".

This commit also adds checking whether specified "from" and "to" nodes
share only one network or not. This should be reasonable restriction
to certainly draw routing lines.

It might be useful to add "network" attribute for inline route
statement, in order to avoid ambiguity of routing line if specified
nodes share two or more networks. But such feature seems low priority,
at least now.
This commit adds Route two additional attributes below.

  - "path" means where routing lines should be drawn

    "la" (left-above), "lb" (left-below), "ra" (right-above), and "rb"
    (right-below) are valid.

  - "pad" means how much routing line should be padded from network lines

    Actual amount of padding at drawing is radius (= diameter // 2) of
    a network trunk multiplied by "pad" value.
Now, each routing lines in "route" statement are drawn.

In this commit, _deautoscaler() is defined as a module level function,
because this can be reusable in other cases, too.
This commit shows that _deautoscaler introduced in previous commit is
useful to centralize similar logic into it, though this commit itself
is not related to "make route statement available".
@flying-foozy flying-foozy force-pushed the make-route-statement-available branch from 9aa3fb8 to 9b334f7 Compare January 30, 2020 13:58
@flying-foozy

Copy link
Copy Markdown
Contributor Author

I re-pushed the series catching up to recent master head.

@jcralbino

Copy link
Copy Markdown

Hello
I want to know if this is released to the production version?
Can we have the latest executables available in GitHub with this feature enabled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants