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.

Sign up for GitHub

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

Jump to bottom

Fix translation warnings in area_end paragraphs #3691

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Fix translation warnings in area_end paragraphs #3691

wants to merge 3 commits into from

Conversation

saviit
Copy link
Contributor

@saviit saviit commented Aug 14, 2024 •
edited
Loading

Since empty area_end paragraphs are not visible, the 'Translation out of date' and 'Check translation' warnings will remain on the area_end pars in translated documents with no obvious way to remove them.

This PR changes the handling of empty area_end pars:

  • the classes troutofdate and checktr will not be set at all for empty area_end paragraphs in new area sections
  • when editing an area start paragraph, the corresponding area_end paragraph is fetched and:
    • it will be given an rt-attribute with the correct hash value
    • its mt-attribute is removed

Bugs:

  • Seems to work for collapsible areas, but not for regular areas
  • Collapsible area headings are no longer marked with the .troutofdate class even when they have not been explicitly translated

Closes #3688

@saviit saviit force-pushed the fix-area-end-tr-attr branch from 5783b14 to d4bc57c Compare August 14, 2024 15:30
@saviit saviit changed the title Fix translation warning in area_end paragraphs Fix translation warnings in area_end paragraphs Aug 14, 2024
@saviit saviit force-pushed the fix-area-end-tr-attr branch from d4bc57c to 2730242 Compare August 15, 2024 15:36
@saviit saviit force-pushed the fix-area-end-tr-attr branch from 2730242 to bf11687 Compare August 20, 2024 07:38
@saviit
Copy link
Contributor Author

saviit commented Aug 23, 2024

Tämä nyt testattavana timbeta02.

dezhidki
dezhidki reviewed Aug 28, 2024
View reviewed changes
timApp/document/editing/routes.py
Comment on lines +331 to +358

# Since empty area_end paragraphs are not visible, we need to set their rt-attribute
# at the same time as the area start paragraph. Otherwise, the 'Translation out of date' and
# 'Check translation' markings will remain on the area_end pars with no obvious way to remove them.
# For new area sections, the classes 'troutofdate' and 'checktr' will not be set at all for the area_end
# paragraphs if it is empty.
if p.is_area():
area_name = (
p.get_attr("area")
if p.get_attr("area")
else p.get_attr("area_end")
)
assert area_name is not None
area_end_par = doc.get_named_section(area_name)[-1]
aep_md = area_end_par.get_markdown()
aep_cache = (
area_end_par.html_cache.get(area_end_par.attrs.get("rt"))
if area_end_par.html_cache
else None
)

if not aep_md and not aep_cache:
res = mark_as_translated(area_end_par)
if not res:
raise RouteException("Paragraph is not a translation.")
# Also mark the area end par as checked since it does not have any content
mark_translation_as_checked(area_end_par)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tarvitaanko tätä enää ollenkaan, jos kerran troutofdate-luokkaa ei enää ikinä aseteta tyhjään area/area_end -lohkoon?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tämä pointti vielä jäi auki. Oliko tälle pätkälle tarvetta, jos tässä PRssä troutofdate poistetaan area/area_end -lohkoista?
Toki jos joskus haluaisi jatkossa näyttää, kuinka paljon on kääntämättä, mutta toistaiseksi varmaan se troutofdate lienee olennaisin poistaa. Ainakin säästyy lisätarkistuksilta lohkojen tallennuksen yhteydessä.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Joo en ole ehtinyt käydä tuota läpi uudelleen, yritän ehtiä tässä melko pian.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tässä taisi olla ideana, että tämä käsittely poistaisi olemassaolevista käännöksistä ne roikkuvat käännösilmoitukset (joita ei tavallisesti editoimalla saa pois). Mutta toki jos käyttää sitä asetusvalikon 'Mark all as translated' toimintoa niin tälle ei tosiaan liene tarvetta.

@saviit
Copy link
Contributor Author

saviit commented Oct 14, 2024 •
edited
Loading

Muutan tämän PR:n draft-statukseen. Tutkittuani tätä tarkemmin, näyttää siltä että käännösmerkintöjen ongelmat eivät korjaannu tällä tavalla yksinkertaisesti, johtuen ainakin seuraavista:

  • atomiset lohkot

  • lohkotuksen ja (erityisesti) alueiden ongelmat, mm.

    • otsikko ei pakota uutta lohkoa (vrt. atomiset lohkot ja #-)
    • sisäkkäiset alueet (toteutus keskeneräinen)
    • alueen päättävä area ja area_end -määritykset yhdistetään automaattisesti niitä seuraaviin otsikkolohkoihin silloinkin kun niille on tehty oma päättävä lohkomäärityksensä #-:llä:
    #- {area="alue"}
## Alueotsikko

#-
tekstiä

#- {area_end="alue"}

## Otsikko 2

    muuttuu muotoon

    ## Alueotsikko {area="alue"}

#-
tekstiä

## Otsikko 2 {area_end="alue"}

    Ylläolevaa tuskin pystyy muuttamaan rikkomatta olemassaolevia dokumentteja (tai niitä varten pitäisi kirjoittaa ja ajaa korjausskripti). Lohkotuksen muuttamisesta taisi jo olla periaatepäätös että atomisia lohkoja ei jatkossa enää tehtäisi automaattisesti, vaan että otsikkotasot muodostaisivat lohkoja samoin kuin nyt tekee #-.

  • esimerkkidokumentti ongelmista: https://tim.jyu.fi/view/users/viitanen-sami/test/translation/area_tr-warnings/a1, erityisesti dokumentin käännös

  • .checktr ja .troutofdate -tyylien määritykset ovat ongelmallisia tietyissä tilanteissa (esim. .checktr asetetaan koko areaContent div:lle, josta se pomppaa ylös dokumentin oikeaan yläreunaan)

Saattaa jopa olla fiksumpaa vaan kuopata tämä PR toistaiseksi kunnes nuo ylläolevat edellytykset on saatettu maaliin.

@saviit saviit marked this pull request as draft October 14, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dezhidki dezhidki dezhidki left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

area_end -lohkoa ei merkitä käännetyiksi kun siihen liittyvä area merkitään käännetyksi
2 participants
@saviit @dezhidki