[PATCH] make toporouter more visible

[Stanislav Brabec]

Toporouter is a very nice but nearly invisible feature. Attached patches make them more visible.

[Peter Clifton (pcjc2)]

I have no idea how these sed expressions are working.. shouldn't they be matching for "%if TOPO_ENABLED" specifically?

if WITH_TOPOROUTER
 sed '/%if/d ;/%endif/d' < ${srcdir}/pcb-menu.res.in >> $@
else
 sed '/%if/,/%endif/d' < ${srcdir}/pcb-menu.res.in >> $@
endif

[Peter Clifton (pcjc2)]

Also, I want to see the patch to add the toporouter to the *menu.res files in at least two pieces.

One should be a no-change transform to the existing *menu.res file to make it *menu.res.in (no functional changes).

The second should be to add the toporouter. to it. That needs to be a visible and review-able change, not lost in a big rename.

[Peter Clifton (pcjc2)]

I'm working on doing the [g]pcb-menu.res.in part of the patch. Your initial patch did not pass "make distcheck", as it broke out of tree builds.

[gpleda.org commit robot (gpleda-launchpad-robot)]

A commit was made which affects this bug
git master commit 481fe9e27df67e93921d20cac95a38c49ea6a664
http://git.gpleda.org/?p=pcb.git;a=commit;h=481fe9e27df67e93921d20cac95a38c49ea6a664

commit 481fe9e27df67e93921d20cac95a38c49ea6a664
Author: Peter Clifton <email address hidden>
Commit: Peter Clifton <email address hidden>

    Generate pcb-menu.res and gpcb-menu.res from ".in" files

    No functional changes here, just a preliminary cleaning before
    adding the topological autorouter to the menus. Doing so requies
    conditional inclusion of certain lines, as the toporouter is not
    always built.

    Based on a patch by Stanislav Brabec <email address hidden>

    Affects-bug: lp-812429

[Peter Clifton (pcjc2)]

Ok, I've committed a patch which makes [g]pcb-menu.res auto-generate from ".in" files, leaving the remainder of your patch as I'm about to attach.

I still think the sed expressions may need work though.

[Stanislav Brabec (sbrabec-suse)]

It would work as long as there will be only a single %if. But yes, it is unsafe, as well as "%endif" alone processing by sed (simple sed expressions cannot pair it with a corresponding %if).

New expression does not try to mimic C syntax.

Here is a new version of the patch:
- It does not modify configure.ac (my first intention was processing it by the compiler, but .res files are read by the runtime, so we need processed res file anyway).
- Adds Makefile dependendcy, so reconfiguring with --disable-toporouter and consequent make work properly.

I still don't know how to prevent need for the click after selecting the Toporouter menu item. (":Toporouter()" from the command does not need it.)

[Stanislav Brabec (sbrabec-suse)]

Description of sed expressions:

if WITH_TOPOROUTER

We want toporouter. Let's delete line starting with "%if_topo" and line starting with "%endif_topo" (i. e. keep the contents of the file including line enclosed inside this %if and just delete these %if lines).

 sed '/^%if_topo/d ;/^%endif_topo/d' < ${srcdir}/gpcb-menu.res.in >> $@

else

We do not want toporouter. Delete everything between line starting with "%if_topo" and line starting with "%endif_topo", including these lines itself (i. e. do not include part inside these %if lines).

 sed '/^%if_topo/,/^%endif_topo/d' < ${srcdir}/gpcb-menu.res.in >> $@

endif

[Bert Timmerman (bert-timmerman)]

Hi all,

I took a stab at this bug/request and have pushed a solution to topic branch "LP812429".

I mainly followed the approach of Stanislav and deviated with the following:

- did not change configure.ac

- used the if-else-endif in Makefile.am to either cat the ?pcb-menu.res.in files to ?pcb-menu.res files ... default

- else cat the ?pcb- menures.in to temp files (?pcb-menu.res.tmp) and then strip the "Toporouter"menu item from the ?menu.res.tmp files and put into proper ?pcb-menu.res files.

This feels a bit like a kludge and probably leave the tmp files cluttering somewhere ;-)

Please review and test.

Kind regards,

Bert Timmerman.

[Stanislav Brabec (sbrabec-suse)]

It works as expected. Toporouter is available in the menu by default and it calls toporouter. With --disable-toporouter, it is not present.

[Bert Timmerman (bert-timmerman)]

AFAICT, the temp files are removed too ;-)