Hooked up Open Text Collection command, multiple related fixes and tweaks

[tjcouch-sil]

Menu -> Project -> "Open Text Collection..." now works

Also

• Added ability to filter to exclude projects from Select Project and Select Multiple Projects dialogs
• Added ability to specify project IDs to start selected on Select Multiple Projects dialog
• Fixed isSerializable to consider value undefined === no key present
• Fixed C# side not being able to return success with no content (if a book/chapter/verse is not present in a project, scrText.Parser.GetVerseUsfmText(verseRef) returns null, which was being overwritten to [] and causing problems)

---

This change is 

[lyonsil]

It will be great to have all of this hooked up!

Reviewed 15 of 15 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @tjcouch-sil)

---

c-sharp/MessageHandlers/ResponseToRequest.cs line 25 at r1 (raw file):

        // Replace null with an empty list to avoid this confusing behavior.
        contents ??= new List<object>();
        return new ResponseToRequest(true, contents);

Are you certain that other cases where we send back "success" with no contents and the TypeScript side of things actually sees it as success? Just wanting to double check as I thought this was something deep inside the network layer in our TS code. It's been many months now, though, and I don't remember for certain.

---

patches/nano-equal+2.0.2.patch line 5 at r1 (raw file):

--- a/node_modules/nano-equal/src/index.js
+++ b/node_modules/nano-equal/src/index.js
@@ -56,9 +56,11 @@ function getType(a) {

I noticed that about a year ago, nano-equal weekly downloads dropped by about 10x. It's in the hundreds now instead of thousands. So we're relying on something that is somewhat obscure and patching it.

Given our recent discussions, it feels like we're doubling down on some changes that were based on my application of programming in other languages that don't have a precise equivalent in TS/JS and a miscommunication between the two of us.

---

src/shared/utils/papi-util.ts line 134 at r1 (raw file):

 *
 */
export function deepEqual(a: unknown, b: unknown, shouldCompareAcrossIframes = false) {

Would it be helpful to add a few small unit tests for this?

[lyonsil]

Reviewed 15 of 15 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @tjcouch-sil)

[tjcouch-sil]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @lyonsil)

---

c-sharp/MessageHandlers/ResponseToRequest.cs line 25 at r1 (raw file):

Previously, lyonsil (Matt Lyons) wrote…

Are you certain that other cases where we send back "success" with no contents and the TypeScript side of things actually sees it as success? Just wanting to double check as I thought this was something deep inside the network layer in our TS code. It's been many months now, though, and I don't remember for certain.

Yeah. I had to go through everything to try to get some context as to why this was being set to []. I intended to make network object get messages respond with an array of function names, but we have never gotten to that. And now I think a lot of our network object rework plans will change all this anyway. Probably should've just let the throw there throw instead of returning undefined, but that's what I decided to do at the time 🤷 so the network object get message must return an array with who cares what in it. I think that's why this was changing null to [], but that should really only happen in NetworkObject.HandleGet, so I moved it there.

In general, requests don't have any demands put on them regarding the contents; if success isn't falsy, it's a success.

[lyonsil]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @tjcouch-sil)

[tjcouch-sil]

Reviewable status: 11 of 20 files reviewed, 2 unresolved discussions (waiting on @lyonsil)

---

src/shared/utils/papi-util.ts line 134 at r1 (raw file):

Previously, lyonsil (Matt Lyons) wrote…

Would it be helpful to add a few small unit tests for this?

I put in a bunch. Hopefully this will help us if/when we ever go back and tune these up.

---

patches/nano-equal+2.0.2.patch line 5 at r1 (raw file):

Previously, lyonsil (Matt Lyons) wrote…

I noticed that about a year ago, nano-equal weekly downloads dropped by about 10x. It's in the hundreds now instead of thousands. So we're relying on something that is somewhat obscure and patching it.

Given our recent discussions, it feels like we're doubling down on some changes that were based on my application of programming in other languages that don't have a precise equivalent in TS/JS and a miscommunication between the two of us.

We decided to put it back to stringify === stringify -> parse -> stringify for now because of various issues with any obvious options.

[lyonsil]

Reviewed 9 of 9 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

---

src/shared/utils/papi-util.ts line 134 at r1 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

I put in a bunch. Hopefully this will help us if/when we ever go back and tune these up.

Awesome, thanks!