Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix download link for binary content (missing namespace) #1854

Merged

Conversation

roland-ruedenauer
Copy link
Contributor

The converter for binary content (moin.converters.everything) in moin version 2.0.0b2 ignores the item namespace when creating the dom content. The result is an invalid download link when viewing the subitem.

Downloads are working when using the fully qualified item name.

Question in this context: shouldn't the link text created for downloading be localized? Currently it seems to hard-coded to "Download <item-name>."

@roland-ruedenauer roland-ruedenauer changed the title Fix download link for binary content (missing namespace) WIP: Fix download link for binary content (missing namespace) Mar 6, 2025
@RogerHaase
Copy link
Member

RogerHaase commented Mar 6, 2025

I think you missed this change to the Development docs:
https://moin-20.readthedocs.io/en/latest/devel/development.html,
https://moin-20.readthedocs.io/en/latest/devel/development.html#install-pre-commit-hooks

install the pre-commit hook:

pre-commit install  # pre-commit is used for code linting / auto-format

and the commit to master is hung up (and rather hidden).

If you click on the red X above and then click on Details you will see Black is complaining about some code formatting.

I think a way to fix this is to:

run the "pre-commit install" command
make a minor change to everthing.py (add a trailing blank), then try to commit
Black will complain about the code format, and correct it automatically
do de-stage, stage cycle
commit, push

@UlrichB22
Copy link
Collaborator

You are right about the localization. There is already a translation for the word “Download”. Perhaps we can use that.

@roland-ruedenauer roland-ruedenauer force-pushed the fix_download_binary_data branch from b643337 to 89c5ed5 Compare March 6, 2025 19:20
@roland-ruedenauer
Copy link
Contributor Author

Sorry, I obviously didn't carefully read your contributor guidance documents and missed the pre-commit hooks.

I've pushed the re-formatted changes (using pre-commit run is the simplest way to trigger the hooks as I found out).
There is now a new translation for the download link text. The already existing "Download" didn't seem suitable to me.

I also tried to update the i18n messages as documented, but found the latest release of pybabel adds lots of new comment lines containing #, python-brace-format and skipped this step.

@roland-ruedenauer roland-ruedenauer changed the title WIP: Fix download link for binary content (missing namespace) Fix download link for binary content (missing namespace) Mar 6, 2025
@RogerHaase RogerHaase merged commit 09931b0 into moinwiki:master Mar 7, 2025
8 checks passed
@RogerHaase
Copy link
Member

Thanks for helping!

@roland-ruedenauer roland-ruedenauer deleted the fix_download_binary_data branch March 10, 2025 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants