m2ms / fragalysis-frontend

The React, Redux frontend built by webpack
Other
1 stars 1 forks source link

Support empty code-prefix in config.yaml #1368

Open phraenquex opened 7 months ago

phraenquex commented 7 months ago

It should be possible to specify the empty string as a code_prefix value. @kaliif can you confirm whether the target loader will handle this?

tdudgeon commented 7 months ago

I made small changes to XCA to allow empty string to be used for code_prefix and code_prefix_tooltip. Now need to check whether target loader can handle this. Note that if empty string is used for code_prefix then no entry for the code_prefix_tooltip will be created.

In thinking this through there is an edge case where if the same code_prefix value (whether empty string or an actual value) is used for multiple inputs then only the code_prefix_tooltip for last one will be recorded. I'm assuming that the code_prefix values are supposed to be unique, but this is not currently enforced. @phraenquex @Waztom your thoughts on this?

kaliif commented 7 months ago

@phraenquex, target loader needs some minor changes to allow empty strings, at it's current state, empty string on code_prefix throws an error

phraenquex commented 7 months ago

Thanks @tdudgeon. You raise a good point - I'd forgotten to think it through, or spec it properly.

So the only way to have different input_descriptors for files from the same source, is to put them in different locations. (Because dir needs to be unique, I guess?)

@kaliif I can imagine this might have an impact on the target loader - or maybe not?

tdudgeon commented 7 months ago

I discussed with @kaliif and we think the best solution is to include the tooltip/input_descriptor in the data for each crystal alongside the code_prefix (rather that have it as a lookup based on the code_prefix value). This will require small changes to XCA and the target loader. This will allow duplicate values for code_prefix and allow the empty string to be used.

@phraenquex can you confirm whether you want this doing for the purple release and that you want code_prefix_tooltip renaming to input_descriptor.

phraenquex commented 7 months ago

Okay, it's correct as implemented. Keep it code_prefix_tooltip.

XCA should issue a warning if it finds different tooltips for the same code_prefix.