Add multiple softwares to application exception window

I've updated the application exception window to allow the user to add multiple software in a list and apply a blanket policy to all of them. You can still add a single item, of course.

Scenario: Some software have several exes which aren't spawned as a child process and therefore I have to manually add them one-by-one into tinywall's application excpetion windows. With the changes I've made, you can add multiple files/software and apply the same policy to all of them.

Next update: To enable multi-select in the browse for process dialogue.

Hello! I reviewed the changes in this PR. Unfortunately there are multiple issues. The examples provided in parentheses below are examples and are not meant to be complete listings of the problematic positions.

It breaks existing localizations (see label4 in ApplicationExceptionForm).
It breaks saved configurations of existing users without giving a migration path (see AppExceptionTimer)
It breaks visual GUI layout, resulting in overlapping and even non-visible controls (try loading the settings window or the exception form on a non-DPI-scaled desktop)
Also GUI layout: The new listbox increases the vertical height of the window by a great amount. The current window already barely fits on smaller screens with desktop scaling. The new window probably spills off the screen edge on such setups.
It breaks existing features (cannot create machine-global exceptions anymore)
It increases developer machine requirements without any benefits (.Net 4.8.1; possibly even raises runtime requirement?)
The changes make APIs accessible that are missing from supported Windows versions (Microsoft.Windows.SDK.Contracts) for what reason?
Adds string(s) that are not localizable (see btnRemoveSoftware_Click)
It changes the copyright text incorrectly (see here about how to interpret the copyright year)

There's also an overdone effort to rename everything to British spelling:

The PR breaks BCL interface contracts (see FilterConditionList.IsSynchroniSed),
The PR changes auto-generated code (see AppFinderForm.InitialiSeComponent).
The PR renames types/variables in code that is supposed to be 1:1 copy of external code (see TaskDialog library) Generally, note that "Dialog" is the actual name of a class of windows in Windows. It is a quasi-standard. The thing's been named, over. This is not a question of spelling anymore, but the question of us using the same references as the creators and basically everybody else on the internet.
The PR renames constants in interop code that are supposed be 1:1 mirrors of native API (see changes in WFP library)
But the main problem of changing the spelling to British is that it is completely subjective. I personally couldn't care less, but what will we do when the next contributor from another country wants to "correct" the spelling? We'll be playing ping-pong between British/Australian/US/Canadian spellings. Besides, many of the new spellings in the current PR aren't even correct by the Oxford Dictionary.

And lastly, there are also some changes that, while maybe not wrong, are at least questionable:

GUI design: The new code allows the user to create a new exception over many applications at the same time. But when the exception is finally created, this grouping is lost. Even though it looks like the user is creating a single rule that spans many applications, they aren't able to edit the same group later on. If the user ever wants to change the rule, they will have to go through and edit each application separately. I'm OK to merge this feature even you don't change this aspect, but think it over once more. Maybe there's a better way to do this.
What's the point of adding parallel-for at rule creation? Are you expecting users to manually select tens of thousands of applications in a single go in a single rule? The expected number is more like a couple, or at most a few dozen. Parallelizing this is counterproductive and useless.

With so many issues I cannot merge this PR. And because the actual feature and completely unrelated edits and refactors are all mixed in a single PR, it is a yes-or-no as a whole. To be mergeable, this PR will need an extensive rebase & do-over.

At a minimum:

Fix all the newly introduced bugs (missing and broken localizations, broken features, broken GUI layout, broken user settings etc... see above)
Forget about the subjective spelling corrections and renames. If you find typos or anything that is objectively wrong, it is okay to correct them. Also feel free to name things you introduce yourself however you like. But we do not want to set a precedent for making subjective ping-pong edits between contributors.
Don't unnecessarily increase development or runtime dependencies/requirements.
Don't do multi-threading where it will only make things worse.
Don't change somebody else's copyright claims.
Don't mix refactors/cosmetic changes in the same commit with functional changes.
Verify new window layout on small high-res screens, and adjust if necessary. Maybe a dropdown instead of a listview?

Ta. Good morning.

Where’s your test harness? Or rather how do you test? I didn’t see any testing project within that solution. Or did I completely miss it?

With regards to the parallel usage, that part of the code always struggled on my machine so using that sped it up.

With regards to the spelling: Oxford English Dictionary is the one that actually provided those fixes; these are fed directly into VS.

Dialogue is what we’ve used in all the commercial entities where I’ve worked at. I’ll be sticking with the English standard over the american one. I’ve never heard of Dialog being a standard in all these years, and I think that’s more to do with people just using it because the SDK was developed in the united states.

I wasn’t aware that the copyright name has changed. I’ll look into that.

What’s a small high-res screen? I only have the one laptop at the moment.

I didn’t understand what you meant by ‘the grouping is lost’ when adding multiple exceptions. Please explain. Thanks.

I’ll add the missing localisation using bing translator.

.NET Framework v4.8.1 was part of the recommendations within VS. I don’t believe I’ve added any new libraries unless they’ve been added indirectly through the 4.8.1 upgrade.

Cheers.

Hi,

Where’s your test harness? Or rather how do you test? I didn’t see any testing project within that solution. Or did I completely miss it?

There isn't any, testing is manual. Yes, this is unfortunate (but not all that uncommon with desktop GUI code). Anyway, there are enough problems with this PR where a test harness wouldn't have helped at all. Stuff like visual layout, DPI scaling issues, missing localizability of new strings, changing dependencies, false copyright adjustment, many of the renames, etc.

With regards to the parallel usage, that part of the code always struggled on my machine so using that sped it up.
Parallel.ForEach(_tmpExceptionSettings, tmpExceptionSetting =>
{
tmpExceptionSetting.Policy = HardBlockPolicy.Instance;
});
The loop body is literally a single class assignment (a pointer assignment+ref counting). That hogged down your laptop? Considering that a person will put at most a couple of dozens of apps in that list, I find it hard to believe this provided a meaningful performance relief. The other few places where parallel-for was introduced have equally trivial bodies and the same number of iterations..

With regards to the spelling: Oxford English Dictionary is the one that actually provided those fixes; these are fed directly into VS.

You put: organisation; Oxford says: organization You put: synchronise ; Oxford says: synchronize You put: serialise; Oxford says: serialize You put: localise; Oxford says: localize ... and so on, to name a few. So contrary to the claims, these don't seem to come from the OED at all. Even if you have automated tools that feed stuff directly into the IDE, you should review the results. You also have to realize that words written with z instead s are not always due to americanism, but due to etymology.

But the Oxford spelling was just a side note. As said the real problem is the precedent this introduces for future contribution. And that of renaming things where we're not supposed to touch them. There was even an interface breakage you introduced (IsSynchronized)! I don't care what English you use for new code. As far as I'm concerned you can even use AAVE and I'll merge it. My problem is changing the spelling of existing code just because you're from another country. The next PR from somebody else will then want to change everything to American and so on and we'll be playing this game for decades.

Dialogue is what we’ve used in all the commercial entities where I’ve worked at. I’ll be sticking with the English standard over the american one. I’ve never heard of Dialog being a standard in all these years, and I think that’s more to do with people just using it because the SDK was developed in the united states.

Of course Dialog's been named like that because americans wrote the code. But that's how it's been named and it is irrelevant why. That's also how you're going to find all SO posts, all API calls, all docs, etc. Even worse, you renamed TaskDialog. That's not only the name of the control here, but also the name of the external library. Do you also rename SQLite in your projects to SQLight because that's the formally correct spelling? You wouldn't, right? Same case here.

I wasn’t aware that the copyright name has changed. I’ll look into that.

The name didn't change. You changed the year incorrectly whereas it should have stayed the same. Updating it only to the current year would relinquish previous claims to existing code. For confirmation and details, please open the link I posted.

I didn’t understand what you meant by ‘the grouping is lost’ when adding multiple exceptions. Please explain. Thanks.

You add an exception for multiple applications, but then you cannot edit the same exception anymore. If you add an exception for 10 apps, you have to make 10 separate edits after that manually.

I’ll add the missing localisation using bing translator.

The problem is not the missing translations. (And please don't add automatic machine translations unless you speak that language and can check the results. Machine-translated results are often wrong for GUIs as there's no context for the software.) The problem is that the string in question was not added in a localizable way. E.g. it didn't use a resource lookup. It is okay if you only add the English version, but even that has to come from a resource lookup like all other strings.

The loop body is literally a single class assignment (a pointer assignment+ref counting). That hogged down your laptop? Considering that a person will put at most a couple of dozens of apps in that list, I find it hard to believe this provided a meaningful performance relief. The other few places where parallel-for was introduced have equally trivial bodies and the same number of iterations..

Actually, it did but this isn't the root cause. I believe it's the UI thread slowing down which is affecting those that I've modified. I'm still investigating but I'm looking at thread pools and queuing work items. I'm also seeing slow down when right-clicking the icon in the tray, which can be up to 12 seconds.

There isn't any, testing is manual. Yes, this is unfortunate (but not all that uncommon with desktop GUI code). Anyway, there are enough problems with this PR where a test harness wouldn't have helped at all. Stuff like visual layout, DPI scaling issues, missing localizability of new strings, changing dependencies, false copyright adjustment, many of the renames, etc.

Okay. It can be done as I've done it in the past with WinForms and WPF. It depends on the effort involved. Ta.

You put: organisation; Oxford says: organization You put: synchronise ; Oxford says: synchronize You put: serialise; Oxford says: serialize You put: localise; Oxford says: localize ... and so on, to name a few. So contrary to the claims, these don't seem to come from the OED at all. Even if you have automated tools that feed stuff directly into the IDE, you should review the results. You also have to realize that words written with z instead s are not always due to americanism, but due to etymology.

I get s instead of z from OED. The hints and linting actually state it's an american preference with the English preference being s.

Of course Dialog's been named like that because americans wrote the code. But that's how it's been named and it is irrelevant why. That's also how you're going to find all SO posts, all API calls, all docs, etc. Even worse, you renamed TaskDialog. That's not only the name of the control here, but also the name of the external library. Do you also rename SQLite in your projects to SQLight because that's the formally correct spelling? You wouldn't, right? Same case here.

Actually... :)

The name didn't change. You changed the year incorrectly whereas it should have stayed the same. Updating it only to the current year would relinquish previous claims to existing code. For confirmation and details, please open the link I posted.

Not at that point in time but I did afterwards. I'll correct the date. Thanks for this and the link.

You add an exception for multiple applications, but then you cannot edit the same exception anymore. If you add an exception for 10 apps, you have to make 10 separate edits after that manually.

They were meant to be individual edits and not grouped. The idea is to add multiple items with the same configuration profile. I've updated the file dialogue to select multiple files now.

The problem is not the missing translations. (And please don't add automatic machine translations unless you speak that language and can check the results. Machine-translated results are often wrong for GUIs as there's no context for the software.) The problem is that the string in question was not added in a localizable way. E.g. it didn't use a resource lookup. It is okay if you only add the English version, but even that has to come from a resource lookup like all other strings.

That's been updated to use the resource strings now. Ta.

I've updated the installer to reference the projects so that I can have it as part of the build pipeline without having to write a script that copies files across to a specific folder. Out of curiosity, why were the files being copied manually into a specific location? Was that some kind of an assurance check on your part?

I'll leave it as a fork since that would be the most sensible approach going forward rather asking you to merge it. Thank you for your advice. Take care.

Actually, it did but this isn't the root cause. I believe it's the UI thread slowing down which is affecting those that I've modified. I'm still investigating but I'm looking at thread pools and queuing work items. I'm also seeing slow down when right-clicking the icon in the tray, which can be up to 12 seconds.

Then there's something else going on. Stuff like that definitely shouldn't take multiple seconds, and TinyWall is good in that regard and behaves snappy. The only known case where the GUI thread slows down is when it is trying to contact the service process but cannot. Then there are timeouts involved with multiples retries. However, fully parallelizing the communication won't help, as it needs the data from the service to show the correct GUI.

But since there's no communication with the service involved when executing those parallel-for loops, this cannot be the reason in your case, and so it is a separate issue from the opening of the tray menu.

Okay. It can be done as I've done it in the past with WinForms and WPF. It depends on the effort involved. Ta.

I know. I haven't put in that effort, that's on me.

I get s instead of z from OED. The hints and linting actually state it's an american preference with the English preference being s.

Oxford has its own 3rd version of the spelling (but still it's one of the most accepted world-wide). English preference writes everything with s, american everything with z. Oxford gives hints about these as you noted. But the Oxford-recommended spelling itself takes different sides in different cases based on etymology.

Even worse, you renamed TaskDialog. That's not only the name of the control here, but also the name of the external library. Do you also rename SQLite in your projects to SQLight because that's the formally correct spelling? You wouldn't, right? Same case here.

Actually... :)

They were meant to be individual edits and not grouped. The idea is to add multiple items with the same configuration profile. I've updated the file dialogue to select multiple files now.

If this is how it's meant to work, I'm OK with that. As already noted previously, this not a blocker for me at all.

I've updated the installer to reference the projects so that I can have it as part of the build pipeline without having to write a script that copies files across to a specific folder. Out of curiosity, why were the files being copied manually into a specific location? Was that some kind of an assurance check on your part?

Mainly because:

I liked the idea of having the installed layout to build the installer from.
I liked the idea that I can mess with different app versions in the build directory without affecting the installer sources.
And it wasn't a big chore, as once the installer layout was built, all the rest of the times it was a single Ctrl-C - Ctrl-V between two folder contents.

If this was to change, I'd have no problem with that either. It's not important at all.

I'll leave it as a fork since that would be the most sensible approach going forward rather asking you to merge it. Thank you for your advice. Take care.

That's a shame. If you ever reconsider, I'm still open to pulling the feature. Maybe we have a different view on the spelling changes, but other than that I don't believe my requests were extraordinary. I mean, other than the spelling it mostly is just fixing the newly introduced bugs.

Then there's something else going on. Stuff like that definitely shouldn't take multiple seconds, and TinyWall is good in that regard and behaves snappy. The only known case where the GUI thread slows down is when it is trying to contact the service process but cannot. Then there are timeouts involved with multiples retries. However, fully parallelizing the communication won't help, as it needs the data from the service to show the correct GUI.

There's something definitely going on here. It also happens with your version that I've got installed. I'll keep investigating but for now Parallel.For are working better than not having them at all. I've also used thread communicate with the service which is working wonders at the moment.

If this is how it's meant to work, I'm OK with that. As already noted previously, this not a blocker for me at all.

I will, of course, improve this going forward but this fills the use case I have.

Mainly because:

I liked the idea of having the installed layout to build the installer from.
I liked the idea that I can mess with different app versions in the build directory without affecting the installer sources.
And it wasn't a big chore, as once the installer layout was built, all the rest of the times it was a single Ctrl-C - Ctrl-V between two folder contents.

If this was to change, I'd have no problem with that either. It's not important at all.

For now, once the projects have compiled. The output is taken from the projects by the Wix project when building the installer. I will look at modifying it so that, the output is copied automatically to a location which the Wix project can copy into itself. to produce the installer. It seems to be working with GitHub actions as well.

That's a shame. If you ever reconsider, I'm still open to pulling the feature. Maybe we have a different view on the spelling changes, but other than that I don't believe my requests were extraordinary. I mean, other than the spelling it mostly is just fixing the newly introduced bugs.

Most of those bugs have been fixed around localisation and UI. I'm still testing but my laptop only goes down to 1080 at 100% scale. I can't test any further than that.

Have a good evening.

pylorak / TinyWall

Add multiple softwares to application exception window #5