feat(Go to file): Autotype suggestions typing path

[chrjorgensen]

Changes

This pull request is a squashed version of PR #1242 and must be used instead to make a cleaner git history.
The original author @sebCIL is kept on the commit.

[worksofliam]

I think there is a lot of logic being handled in the wrong file.

I am thinking code-for-ibmi.goToFile (and code-for-ibmi.goToFileReadOnly) should likely go into a separate file.

Secondly, I think we need to be careful about using onDidChangeValue because a fast typing user is resulting in spamming the server. We need to look into using some kind of timeout to slow down the requests to the server.

Something like this perhaps?

quickPick.onDidChangeValue(async () => {
  clearTimeout(timeout);
  timeout = setTimeout(() => {
    // Do something with quickPick.value
  }, 500);
});

[chrjorgensen]

@worksofliam

Secondly, I think we need to be careful about using onDidChangeValue because a fast typing user is resulting in spamming the server. We need to look into using some kind of timeout to slow down the requests to the server.

Ah - I didn't understand your comment until I realized that onDidChangeValue would call the server at any time there was a * in the typed value. This was an error, it was only supposed to call the server when the typed value had a '*' in the end. I have fixed this in commit 574b2bd.

I have also fixed some other small issues - can I make you test it one more time and see if it's better now?

I'm fine with the code being extracted into a separate file - but can we do that in 2.4.1? I will make an issue about this, if you are okay with this.

[chrjorgensen]

@worksofliam Also note that there's a limit of 30 members retrieved from the server, in order to not delay the list unnecessary...

[worksofliam]

@chrjorgensen

I'm fine with the code being extracted into a separate file - but can we do that in 2.4.1? I will make an issue about this, if you are okay with this.

Perfect. Thank you! I will have another review for you today 💯

[worksofliam]

Consider not adding this condition against TABLE_PARTITION when filterText is blank. It's evaluating to: TABLE_PARITION like '%'.

QSYSINC/H/*

[worksofliam]

Same here about filterText being blank.

[worksofliam]

Functionally works good. I left some comments about the where clause being used for the filter. I also think we need an improved UX to show that it is loading data, perhaps with something like this before the SQL statements are executed?

                quickPick.items = [
                  {
                    label: 'Searching',
                    alwaysShow: true,
                    description: 'Please wait..',
                  },
                ]

[worksofliam]

@chrjorgensen Here is a summary of my changes, which are quite a lot.

• You can no longer select the 'Loading' indicator
• Stopped passing in a clause for filterValue if it is empty and limit the results based on the filterValue variable (see maxItems constant)
• Removed forEach and push combination in place of map on all result sets.
• Removed the previous caching mechanism due to the fact that I couldn't backtrack my filter, for example:
  1. If I filtered against QSYSINC/MIH/*, it would show me the members for that source file - great!
  2. In the same quick pick, if I change the value to QSYSINC/H/* then nothing would load.
• Keep the limit to 30 if the filterValue is empty, otherwise the limit is very high as the user is being more precise.
• After the filter items have loaded, remove the * so the user can filter the filtered items (filter-caption?!?!)

This is obviously a lot of changes, but think I did what you would think is right. If not, I am happy to back out my changes.

[chrjorgensen]

@worksofliam Your changes worked great - but there were some old errors and other stuff, that I changed:

• Searching for names with variant characters did not work - fixed.
• I removed the limit on the SQL (maxItems constant), since it's fast enough even without the limit. 14000+ members were retrieved in approx. 5 seconds on my small test system.
• The typed value should not be added to the list (a leftover from copying from the Change Current Library function, I guess).
• I changed the search instruction to be more clear.

Please have a go. If you're pleased with it, let's make the release - we can always fix any small errors later... 😃

[worksofliam]

Ok. This is soooo cool. I love this feature!!! Excellent work @chrjorgensen - thanks for your awesome work on this. Let's merge and I will release.

Things I want to do in the future:

• Move the source code for this into another .ts file
• Perhaps if I enter QSYSINC (the library) there can be an option to create a filter from the list

[rbeckham]

just got to try this new feature. This is so awesome!!! this was my biggest complaint with Code for i, such a great enhancements. Thanks to all who contributed!