cbi: fix to work inside/along with other std directives

[ianchi]

Currently the directive is not working inside/along with other std directives (such as ngRepeat or with {{variable}}). The initialization is done at compile or pre-link traversing the children, but they are not yet linked (if they are not static).

Change initialization to linking phase and change logic to not traverse the children. Instead each children calls the Map at the moment of initialization.

While at it, move global regex variables to local scope, as now they are only needed there.

Signed-off-by: Adrian Panella ianchi74@outlook.com

[jow-]

The whitespace / indentation looks broken, did you maybe mix tabs and spaces?

Another problem I see is that the uci configs are not loaded batch-wise anymore but that shouldn't be a big blocker. Will review it in more detail later

[ianchi]

Regarding whitespaces, I was trying out a new editor for js (Brackets) and perhaps it messed things up (or Probably I messed up the configuration...)

Regarding the batch calls, it was a conscious trade off. To call batch you need to know all config files involved. But to have them while allowing dynamic markup, you should wait till all the page is rendered then do the call, wait for results and complete init. As I think that one view generally involves only one config set in cbiMap, or one or two more at most, I thought that in the general case it would be better to fire the first call before the linking (in Map pre) so that in general it would be available for the rest, and if any other config was needed call them on demand. Besides other visited views might already have called that shared Config and so could already be in l2rpc cache.

[ianchi]

An off topic question, what is the roadmap for luci? Is it Luci2 or is the Luci2-ng fork the LEDE 'official' approach? (I ask as the project is not included in the Lede repisitories) I like this angular approach, and really think that LUCI is in good need of a refresh.

[ianchi]

corrected my editor's config options and fixed mixed whitespaces.

[jow-]

Merged, thank you! I agree that most CBIs map only one or two configs, so the benefits of your approach far outweigh the little overhead of not batching the load anymore.

You can consider this luci-ng fork to be the canonical implementation of a client side luci. Luci2 was a proof of concept used to develop the backend services while luci-ng aims to be a proper ui eventually

[jow-]

@ianchi - I was revisting this code while testing your other PR for bumping Angular and noticed that it completely broke the waitfor / init / finish cycle of CBI widgets. One example where you can see the breakage is on the static routes configuration page - the cbiNetworkList finish function is called before l2network.load() was executed, means the interface dropdown displays Loading... in its caption which is supposed to be replaced with the effective value in finish().

The idea was that the .finish() finalizers of Maps/Sections and Options are only called once all .waitfor() requests have finished.

Do you have any idea how we can easily defer calling .finish until all .waitfor() calls have been both enqueued and finished?

[ianchi]

@jow- I'll definitely look into it and share any ideas. Maybe we can do something in the postLink callback

[ianchi]

@jow- I've been looking into this and have some coments/doubts:

• it'll be extremely difficult to enable the use of dynamic content and at the same time make the cbiMap know when every single child directive has been linked, and so all possible waitfor have been enqueed. PostLink function doesn't warrantee that there won't be any deferred child linking
• so far I've only seen the use of wait for to filling combos (devices and protocols) Is there any other use case?

I think that if it is only for that, the code should be reworked so each control is autonomous and waits on its own relevant events, instead of building a very complex hierarchical wait structure that relies on top down knowledge.

In spite of that, we could leave the waitfor functionality (to allow to wait for external events) but instead of calling everything only once centralized from cbiMap, each control should wait for the promise in its own waitfor attribute and on its immediate parent (which in turns waits it's own parent). Having everything decentralized should allow any arbitrary nesting of dynamic controls. But still, I'm not quite sure all this complexity is really needed. Perhaps only at cbiMap level to wait for data at the controller level.

If we agree on the intended behavior I can send a PR with a proposal for the changes.

Sorry for the lengthy post, and for being so disruptive again.

[jow-]

Yeah, I also think we should localize the waitfor handling and I have no real problems with changing it. The only thing we must ensure is to block the cbiMap / Make it readonly while wait operations are pending. Maybe, upon calling waitfor(), we should disable the ownerMap buttons or display a loading data overlay.

I basically want to avoid a case where a user hits "Save" while some input widgets are still initializing.

[ianchi]

Ok, I have already a modified version with localized waiting where static routes page is working again. I'm still testing the "save" workflow.

I have a doubt there. Why do you call a "reset" of the sections just after saving it? Shouldn't data stay the same?

Another doubt. I haven't completely followed all the underlying calls, but when calling save & apply as two consecutive functions that in turn call two async rpc calls independently how can you be sure that there is no race condition in the order of the calls, and that "apply" only acts after "save" has finished? Is it somehow handled in l2.rpc or uci itself? Shouldn't "apply" be chained with a "then" after save (which should be made to return the promise)?

I will look into the blocking feature and send the changes in a PR for your revision/comments.

[ianchi]

Pushed the proposed changes in PR #25

[jow-]

I think I added the reset after save to force the local render state to stay in sync with whatever is on the remote device. It might also be necessary to properly redraw some things but I am not 100% sure anymore.

The apply code path was a quick hack to get things working, it needs to be completely rebuilt to be properly synchronized. I also wanted to make it use of the apply/rollback facilities offered by rpcd to be able to react on problems.