imagej / imagej-legacy

ImageJ+ImageJ2 compatibility layer
https://imagej.net/libs/imagej-legacy
BSD 2-Clause "Simplified" License
16 stars 25 forks source link

Expand plugins-config support #266

Closed hinerm closed 3 years ago

hinerm commented 3 years ago

We were missing coverage of two valid plugin definitions in plugins.config:

This patch adds support for both.

Closes https://github.com/imagej/pyimagej/issues/133

hinerm commented 3 years ago

@ctrueden there may still be commands missing. I tried identifying them by looking at all the mismatches during plugin parsing but only found empty lines and menu separators.

Plugin counts (per the menu when starting Fiji):

Not clear if these empty lines/menu separators account for the discrepancy.

In any case I think we should merge this as-is and wait to see if other commands are identified as missing.

ctrueden commented 3 years ago

@hinerm Awesome! Now I understand why some commands are reliably missing! I had forgotten that I wrote that plugins.config parsing to finally fix once and for all (or so I thought, ha ha) the plugins.dir problem. Thank you very much for tracking this down!

I will take a quick look at which commands are present versus missing—just dump the lists and sort and diff them, to see where the regex is falling short. If easy to fix, will fix, and if less obvious or clear, just merge this as is.

ctrueden commented 3 years ago

I read in all the plugins.config files and concatenated them:

import imagej
from scyjava import jimport

ij = imagej.init('sc.fiji:fiji:LATEST')

BufferedReader    = jimport('java.io.BufferedReader')
InputStreamReader = jimport('java.io.InputStreamReader')
Thread            = jimport('java.lang.Thread')

cl = Thread.currentThread().getContextClassLoader()
pluginsConfigs = cl.getResources("plugins.config")

lines = []
while pluginsConfigs.hasMoreElements():
    r = BufferedReader(InputStreamReader(pluginsConfigs.nextElement().openStream()))
    while True:
        line = r.readLine()
        if line is None: break
        lines.append(line)

As of fiji 2.0.0-pre-111, the resulting list is 1270 lines long.

Then I pruned all comment lines and whitespace-only lines:

plines = [str(line).strip() for line in lines]
commands = [line for line in plines if not line.startswith('#') and line != '']

The list is then 406 actual command declarations.

Then I filtered the list to only those not matched by your new-and-improved pattern:

Pattern = jimport('java.util.regex.Pattern')
pattern = Pattern.compile("^\\s*([^,]*),\\s*\"([^\"]*)\",\\s*([^\\s]*(\\(.*\\))?)\\s*")
invalid = [command for command in commands if not pattern.matcher(command).matches()]
>>> print('\n'.join(invalid))
Plugins>Image5D, "-"
Plugins>Image5D, "-"
Plugins>Image5D, "-"
Plugins>Image5D, "-"
File>Import, "-"
Plugins>Bio-Formats, "-"
Plugins>Bio-Formats, "-"
Plugins>Bio-Formats, "-"
Plugins>Bio-Formats, "-"
Plugins>Bio-Formats, "-"
Analyze>QuickPALM, "-"
Analyze>QuickPALM, "-"
Analyze>QuickPALM, "-"
Analyze>QuickPALM, "-"

And we can see it's only separators (14 of them).

So I think your new pattern fully covers all the valid plugins.config lines! :+1: If there are any other missing commands, it will be because of some other reason besides bad plugins.config parsing.

1 I do not know why, but asking for sc.fiji:fiji:LATEST, rather than downloading the latest SNAPSHOT version, instead yields fiji-2.0.0-pre-11.jar. Seems like a clear bug to me, but since the version was consistent for testing the regex for now, I just went with it.