Bit-Slicer icon indicating copy to clipboard operation
Bit-Slicer copied to clipboard

feat(label): add ZGLabel and ZGDocumentLabelManager implementations

Open wesselvdv opened this issue 1 year ago • 7 comments

This should at least resolve #66 and adds a starting point for #16.

No tests yet, as I think some discussion on the exact implementation is advisable.

I am not sure what the best approach is for solving #66. I see two possibilities with this PR:

  1. Separate a ZGLabel and ZGVariable in the UI, and create separate tables for them to manage them. A ZGLabel is referencing a ZGVariable, and as such the table implementation can be re-used for ZGLabel. When assigning a label to a variable, you would actually lift the referenced variable into a label and this means that the variable is moved from the variables list to the labels list. You can still interact with the variable in the same way, but it now is a label for all intents and purposes. (e.g. edit address formula and others)

  2. Do not create separate tables for variables and labels, instead augment the variables view in some way that adds a simple but clear distinction between variables and variables with labels. This would be the least amount of work, but I am not sure what the best way to indicate a label on a variable would be. An additional column is easiest, but is also a waste of space. (Assuming most variables don't have a label.) The creation of a label is the same as for 1. (e.g. lifting the ZGVariable into a ZGLabel). But it'll stick around in the variables array, and still be part of the variables table.

wesselvdv avatar Jan 22 '23 18:01 wesselvdv

Thank you for the PR. Left some small comments for now but will take a closer look and think about this more.

zorgiepoo avatar Jan 23 '23 02:01 zorgiepoo

Thank you for the PR. Left some small comments for now but will take a closer look and think about this more.

I don't see any comments? Or am I stupid.

wesselvdv avatar Jan 23 '23 07:01 wesselvdv

They should show up as inline comments here on GitHub's review.

zorgiepoo avatar Jan 23 '23 07:01 zorgiepoo

I don't see any review, maybe it's still pending? As in, did you confirm the review? Not sure if that's required or not. But I just did a test review, and I do see those comments.

wesselvdv avatar Jan 23 '23 08:01 wesselvdv

After some thinking, I think a variable needs to have a label in order for that label to be around. Basically a label needs to be attached to some variable in the table.

From a user point of view (and not implementation point of view): when a user wants to set a label onto a variable, that should just label the variable with a specific reference-able name. When the user edits the address formula of that variable, the description there (that explains base() and []s) can maybe indicate the reference-able label name of the variable (I believe I had a branch somewhere that actually did something like this). (Edit: the variable description can be updated too if the user hasn’t edited the description themselves yet and it has been so far auto-generated). Also changing the address formula of a variable that is labeled should update the variable/label appropriately. Any other variable can reference the label using label("myLabelName"). When a variable that has a label is removed, that label should also be discarded (unless maybe there's another variable with the same label like a copy but I’m not sure this should be allowed. There is undo support to think about too.). The only labels that need to be updated/managed are labels that are assigned to variables in the table.

From scripting, the user should only be able to set/change an existing label, not add or remove labels.

I don't want an ordered set of labels view the user needs to manage if I can avoid it.

Is it necessary that a label creates another variable? Maybe not. Is it necessary for the document to save another array of labels, perhaps not either.

zorgiepoo avatar Jan 29 '23 07:01 zorgiepoo

Interesting, that was my initial approach (e.g. label property on ZGVariable). You'd still need the ZGLabelManager to keep track of labels that are floating around as a lookup table between label ~> ZGVariable. Regarding the duplicates, you can detect these easily with the ZGLabelManager as it's being stored in a dict there. The UI would have to be added to facilitate the creation of a label then. Another option in the right-click menu perhaps?

wesselvdv avatar Jan 29 '23 19:01 wesselvdv

Regarding copies, I don't think two variables currently in the table should be allowed to be assigned to the same label. So copying a variable that is labeled should likely strip the label. Trying to set the same label onto another variable should fail.

Probably a Edit Variable Label… menu item in the Variable menu from the menu bar and a contextual (control/right click) menu is needed.

zorgiepoo avatar Jan 30 '23 00:01 zorgiepoo

I merged an implementation for adding labels in #95 (more details there). This PR helped me speed up writing an initial implementation.

zorgiepoo avatar May 06 '24 03:05 zorgiepoo