Script completer minor refactoring

[zas]

Summary

• This is a…
  • Bug fix
  • Feature addition
  • Refactoring
  • Minor / simple change (like a typo)
  • Other
• Describe this change in 1-2 sentences:

Problem

• JIRA ticket (optional): PICARD-XXX

Solution

Action

Additional actions required:

• Update Picard documentation (please include a reference to this PR)
• Other (please specify below)

[zas]

Note: 4eb1e2f changes the completer behavior, was there a good reason to use Unfiltered mode?

@phw @rdswift Please test and tell me what you think

[rdswift]

Looks good to me. Everything seems to work as expected in my testing. Is this something that we would also consider porting to the 2.x branch?

[rdswift]

Maybe a dumb question, but is there a reason that we don't support tooltips in the script editor for EXTRA_VARIABLES and PRESERVED_TAGS like we do for TAG_NAMES? Would we be open to a change to support these lists as well?

[zas]

Maybe a dumb question, but is there a reason that we don't support tooltips in the script editor for EXTRA_VARIABLES and PRESERVED_TAGS like we do for TAG_NAMES? Would we be open to a change to support these lists as well?

I see no good reason, it may help users, feel free to open a PR

[zas]

Looks good to me. Everything seems to work as expected in my testing. Is this something that we would also consider porting to the 2.x branch?

Did you compare the behavior between Unfiltered mode (current master) and normal mode (this PR) for the the popup? I wonder if there are cases where previous behavior is better, and why unfiltered mode was chosen to start with.

[rdswift]

Did you compare the behavior between Unfiltered mode (current master) and normal mode (this PR) for the the popup?

I'll check again, but when I originally tested I didn't see any difference.

[rdswift]

Did you compare the behavior between Unfiltered mode (current master) and normal mode (this PR) for the the popup?

I'll check again, but when I originally tested I didn't see any difference.

Oops. I just checked again and I see that the selection list in the original version showed everything all the time but only moved the selected item highlight as you typed, and your version actually reduces the selection list. I'm not sure why it was set up initially as unfiltered, but I prefer the way that you have it set up as filtered.

[phw]

Note: 4eb1e2f changes the completer behavior, was there a good reason to use Unfiltered mode?

The behavior is different. In unfiltered mode it jumps to the suggestion, but shows all the possible completions. As the list is sorted alphabetically and matching looks at the start I found this more convenient to be able to look through all possible functions. But that's probably just my personal preference.

Maybe a dumb question, but is there a reason that we don't support tooltips in the script editor for EXTRA_VARIABLES and PRESERVED_TAGS like we do for TAG_NAMES? Would we be open to a change to support these lists as well?

The tooltips work on all variables. Actually the code to show the tooltip just searches for the percentage sign and valid characters afterwards. So it even works for any user defined variable. Only that for variables we don't have much to show, so it only shows the name. This is different for functions, where we show the function documentation.

[rdswift]

I found this more convenient to be able to look through all possible functions. But that's probably just my personal preference.

I can live with it either way.

[phw]

@zas I think we need a ticket and put the change of auto complete behavior into the changelog.

[zas]

@zas I think we need a ticket and put the change of auto complete behavior into the changelog.

I'll do, and introduce the change in another PR, I removed this change from this one as it was intended to get feedback on it anyway.

[phw]

LGTM