Done. Find at https://invent.kde.org/frameworks/networkmanager-qt/-/merge_requests/2

Changed the help message

Feel free to drop this you think it is not useful. Not a very important development.

In D26917#600766, @cfeck wrote:

Thanks for the investigation! There are more places where m_addressInput is used without a nullptr check, but I didn't analyze the code flow if these can be reached when the m_addressInput widget is not present. Feel free to verify :)

In D21296#467329, @jgrulich wrote:

Looks good.

I addressed your comments. Sorry for the careless coding in what spaces are concerned. BTW, I added a missing QStringLiteral.

I believe I addressed all of your comments. Please check your email for testing options.

Update per comments above

! In D18394#445411, @jgrulich wrote:

Hopefully there are no more trailing spaces.

In D18394#444795, @jgrulich wrote:

1. I'm not sure if the UI for openconnect tokens is correct, I think the QLineEdit for token secret should be on the same line, and you should probably use PasswordField instead? It can be our PasswordField widget from libs/editor/widgets/. Or it's not secret in the same sense as other secrets and it will not need to be saved by secret agent, like rest of passwords? I would also follow nm-connection-editor and make tokens options visible in the main UI, not under specific button.

Diff updated, but please note that this patch would break compatibility with NetworlManager, and with existing configured networks. I still think changes are reasonable, but had to be synchronized with similar changes in NetworkManager. Not that those are difficult, I may be able to implement them, but need to be done. Until all parties agree, I'd recommend not implementing this

That is correct. It was fixed with a different approach

Changed tooltips

Changed tooltips
No need for a new enum

• Removed updateLog calls
• Properly initialize d->tokens
• Change the config dialog so that (a) 'Apply' does not become active when the user clicks on the 'Show Tokens' button, makes no changes, and closes the dialog or clicks Cancel, and (b) when the user clicks 'Cancel', inputs read from the config file are restored
• Added minimal tooltips (should be improved)

Corrects minor bug in one call to updateLog. However, all calls to updateLog have to be reviewed if this goes to production

Changed the buildup of the "tokens" ComoBox

Amended to remove global tokenModeList

I addressed your comments, except the ones regarding macro definitions, since I am unsure thar applicable versions of openconnect have compatible parameters. I will find out and revisit the comment later

Now it builds.

I implemented all of your comments, reverting to my previous approach

Treat otp-flags the same way NetworkManager-fortisslvpn does

I see. Maybe do not set it at all if one-time password is not used and let NM to set the default value? That way we won't be using a wrong value in case this changes in future.

Fair enough. I will update the diff with this. However, judging from the current behavior, if we do not set the value, nothing will be added to the config file. No harm will be made, though.

Removed setting the otp-flag in case it is not needed for the connection

I see. Maybe do not set it at all if one-time password is not used and let NM to set the default value? That way we won't be using a wrong value in case this changes in future.

I implemented all of your comments except one. I created a new VPN setting wit nm-connection-editor, and it sets it to 0, that is, to NetworkManager::Setting::None. Thus, I believe that part is correct.

Treated otp-flags the same way as password-flags. But we have to be careful to consider the case where the option is not yet in the config file (as will certainly be the case the firs time it is used if there was a vpn already configured), so I added a isEmpty() test not to break the toInt() call.

You are, of course, correct. How can I have missed that?