PICARD-3061: Add long descriptions to tags, display them in tooltips #2636

rdswift · 2025-04-16T20:04:42Z

Summary

This is a…
- Bug fix
- Feature addition
- Refactoring
- Minor / simple change (like a typo)
- Other
Describe this change in 1-2 sentences: Add tooltip descriptions to PRESERVED_TAGS and EXTRA_VARIABLES

Problem

The tooltips shown for the tags and variables in the script editor only showed information for the TAG_NAMES. There were no tooltip descriptions for PRESERVED_TAGS and EXTRA_VARIABLES.

JIRA ticket (optional): PICARD-3061

Solution

Add tooltip descriptions to PRESERVED_TAGS and EXTRA_VARIABLES. Add test for tooltips on tags and variables.

Action

Additional actions required:

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

Since this change will result in some new and changed string for translation, please review the new and existing tag description strings to ensure that any desired changes are captured prior to merging this PR.

phw

Generally I agree with the goal of providing more context for the variables. But I don't think the current approach works, as it is repurposing the tag name feature. Instead there should be a separate longer description. See my more detailed comments on the code.

As a side note I think the strings used for description should ideally be the same as used in the documentation. Maybe not always the full text. But the more similar they are (ideally identical) the easier it is to reuse the translations on Weblate.

picard/util/tags.py

rdswift · 2025-04-23T16:35:14Z

Rebased and pushed with significant changes from the original PR. Please review the descriptions and tooltip code, along with identified TODO: entries. We should remove all of the TODO: items before merging.

EDIT: I also tried to put all of the tags in alphabetical order for ease of maintenance and adding new tags. Please request a change if you spot something in the wrong place.

zas · 2025-04-23T17:16:53Z

@rdswift what do you think about displaying the tooltip on tag's titles in metadata box, as done in rdswift#19 ?

picard/util/tags.py

rdswift · 2025-04-23T22:19:46Z

what do you think about displaying the tooltip on tag's titles in metadata box, as done in rdswift#19 ?

Great idea. I merged your changes. Thanks.

rdswift · 2025-04-23T22:37:32Z

Also, just a "heads up" that it looks like the Ubuntu test-requirements isn't loading the markdown module, so I ended up checking for that and providing alternate test results for some of the tests in test_util_tags.py in order to make the CI process happy. The macOS and Windows versions seemed to load okay and passed without the alternative results.

zas · 2025-04-23T23:58:23Z

Also, just a "heads up" that it looks like the Ubuntu test-requirements isn't loading the markdown module, so I ended up checking for that and providing alternate test results for some of the tests in test_util_tags.py in order to make the CI process happy. The macOS and Windows versions seemed to load okay and passed without the alternative results.

markdown is optional, so we should improve matrix testing so we test with and without it.

rdswift · 2025-04-24T01:50:39Z

markdown is optional, so we should improve matrix testing so we test with and without it.

In that case, the upside is that I've covered both conditions in the testing for this PR. 😉

phw · 2025-04-24T07:04:07Z

picard/util/tags.py

    ),
    TagVar(
        'media',
        shortdesc=N_('Media'),
+        # TODO: Confirm this is a multi-value.
+        longdesc=N_('A multi-value tag containing the media on which the release was distributed (e.g.: CD).'),


This isn't multi-value. This is set per track, and a track only has a single media type

phw · 2025-04-24T07:04:50Z

picard/util/tags.py

    ),
    TagVar(
+        # TODO: Check if this actually exists or if it was provided by the last.fm plugin.


"Mood" is one of the defined and mapped tags

picard/util/tags.py

phw · 2025-04-24T08:43:28Z

markdown is optional, so we should improve matrix testing so we test with and without it.

We already have this, the test-requirements tests are specifically for this. They test with reduced or older dependencies. E.g. the first test it runs does install the oldest versions we claim to support, and does not include any optional dependency. The other tests include specific versions of one of the optional dependencies. Currently this is now mostly used to test for alternative packages we support (discid vs. libdiscid, chardet vs. charset-normalizer).

So the purpose here is exactly to catch cases where tests fail with optional dependencies missing. Worked as intended :)

The "normal" tests run with requirements.txt applied fully.

zas · 2025-04-24T09:36:06Z

We already have this, the test-requirements tests are specifically for this.

Thanks for clarifying, I thought something wasn't working, but actually that's the reverse, it is working perfectly :)
@rdswift : since markdown is optional, you have to handle both cases in the code (as you discovered), that's not a problem with tests (a contrario).

picard/util/tags.py

zas

LGTM

zas · 2025-04-27T21:48:25Z

@rdswift do you have more changes to add to this PR? I think it is now (very) good, so I approved it.
I'd like @phw to review it too before any merge, there are more than expect changes (for the good imho) in this PR so extensive review is needed.

rdswift · 2025-04-27T22:30:43Z

@rdswift do you have more changes to add to this PR? I think it is now (very) good, so I approved it.

Nothing more from me on this PR. I may have some more description changes, or link / see also / option items to add, but I will start a new PR for that.

I'd like @phw to review it too before any merge, there are more than expect changes (for the good imho) in this PR so extensive review is needed.

I'd like to have him review it again as well. One thing we still didn't finalize is whether or not to use "No description provided" or just repeat the tag name as the tooltip if there is no longdesc or shortdesc or the tag is not found in TagVars.

picard/util/tags.py

- Add long descriptions to use for tooltips. - Add tooltips method. - Add basic html if markup not loaded.

Add backticks around date format. Co-authored-by: Laurent Monin <lmonin.zas@gmail.com>

@zas

- Add a constant to PICARD_URLS for `https://musicbrainz.org/doc/` - Add a helper to get a title for an option setting in `picard/profile.py` - Only include the long description and notes in tag tooltips. - Add a new method to get a tag's full description including the long description, notes, related settings and links. This could be used to provide tag documentation similar to scripting function documentation. - Change tag options to `is_*` for consistency. Use properties `not_*` for use with `ATTRIB2NOTE` as proposed by @zas. - Update tests.

* Notes/sections: make the code more generic, and easier to extend - use an IntEnum to identify sections - use a namedtuple to describe each section: title & function to get content - replace _add_section() by a more generic _add_sections() * Move `_gen_sections()` to TagVar, make format translatable * Add tests for basic TagVar

- Remove `profile_setting_title()` from `picard/profile.py` - Add `get_option_title()` to `picard/options.py` - Update `picard/util/tags.py` to use new `get_option_title()` function - Add new `test/test_options.py` file - Update `test/test_util_tags.py` file to use new `get_option_title()` function

zas

LGTM

rdswift added the Feature label Apr 16, 2025

rdswift requested review from phw and zas April 16, 2025 20:04

rdswift force-pushed the additional_variable_tooltips branch from 6fb64b9 to cd2d916 Compare April 16, 2025 20:14

rdswift marked this pull request as draft April 16, 2025 20:17

rdswift force-pushed the additional_variable_tooltips branch from cd2d916 to bf70857 Compare April 16, 2025 20:33

rdswift marked this pull request as ready for review April 16, 2025 20:38

phw requested changes Apr 17, 2025

View reviewed changes

picard/util/tags.py Outdated Show resolved Hide resolved

picard/util/tags.py Outdated Show resolved Hide resolved

picard/util/tags.py Outdated Show resolved Hide resolved

picard/util/tags.py Outdated Show resolved Hide resolved

zas mentioned this pull request Apr 17, 2025

Introduce TagVar & TagVars classes #2637

Merged

7 tasks

rdswift marked this pull request as draft April 18, 2025 16:36

rdswift force-pushed the additional_variable_tooltips branch from 68f2ea9 to 535dd59 Compare April 23, 2025 15:01

rdswift requested a review from phw April 23, 2025 15:28

rdswift marked this pull request as ready for review April 23, 2025 15:28

rdswift force-pushed the additional_variable_tooltips branch from 0262cbe to 20ec4f2 Compare April 23, 2025 16:24

zas changed the title ~~Add tooltip descriptions to PRESERVED_TAGS and EXTRA_VARIABLES~~ Add long descriptions to tags, display them in tooltips Apr 23, 2025

zas requested changes Apr 23, 2025

View reviewed changes

picard/util/tags.py Show resolved Hide resolved

picard/util/tags.py Outdated Show resolved Hide resolved

picard/util/tags.py Outdated Show resolved Hide resolved

phw reviewed Apr 24, 2025

View reviewed changes

zas requested changes Apr 24, 2025

View reviewed changes

zas reviewed Apr 24, 2025

View reviewed changes