diff --git a/crates/i18n-scan/src/key.rs b/crates/i18n-scan/src/key.rs index 723177a3..8e4c7118 100644 --- a/crates/i18n-scan/src/key.rs +++ b/crates/i18n-scan/src/key.rs @@ -13,10 +13,12 @@ // limitations under the License. use mas_i18n::{translations::TranslationTree, Message}; +use minijinja::machinery::Span; pub struct Context { keys: Vec, func: String, + current_file: Option, } impl Context { @@ -24,9 +26,14 @@ impl Context { Self { keys: Vec::new(), func, + current_file: None, } } + pub fn set_current_file(&mut self, file: &str) { + self.current_file = Some(file.to_owned()); + } + pub fn record(&mut self, key: Key) { self.keys.push(key); } @@ -39,6 +46,28 @@ impl Context { let mut count = 0; for translatable in &self.keys { let message = Message::from_literal(translatable.default_value()); + + let location = translatable.location.as_ref().map(|location| { + if location.span.start_line == location.span.end_line { + format!( + "{}:{}:{}-{}", + location.file, + location.span.start_line, + location.span.start_col, + location.span.end_col + ) + } else { + format!( + "{}:{}:{}-{}:{}", + location.file, + location.span.start_line, + location.span.start_col, + location.span.end_line, + location.span.end_col + ) + } + }); + let key = translatable .key .split('.') @@ -48,12 +77,23 @@ impl Context { None }); - if translation_tree.set_if_not_defined(key, message) { + if translation_tree.set_if_not_defined(key, message, location) { count += 1; } } count } + + pub fn set_key_location(&self, mut key: Key, span: Span) -> Key { + if let Some(file) = &self.current_file { + key.location = Some(Location { + file: file.to_owned(), + span, + }); + } + + key + } } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -62,15 +102,26 @@ pub enum Kind { Plural, } +#[derive(Debug, Clone)] +pub struct Location { + file: String, + span: Span, +} + #[derive(Debug, Clone)] pub struct Key { kind: Kind, key: String, + location: Option, } impl Key { pub fn new(kind: Kind, key: String) -> Self { - Self { kind, key } + Self { + kind, + key, + location: None, + } } pub fn default_value(&self) -> String { diff --git a/crates/i18n-scan/src/main.rs b/crates/i18n-scan/src/main.rs index ff6dc347..23638e0d 100644 --- a/crates/i18n-scan/src/main.rs +++ b/crates/i18n-scan/src/main.rs @@ -82,6 +82,7 @@ fn main() { let template = std::fs::read_to_string(&path).expect("Failed to read template"); match minijinja::parse(&template, relative.as_str()) { Ok(ast) => { + context.set_current_file(relative.as_str()); minijinja::find_in_stmt(&mut context, &ast).unwrap(); } Err(err) => { @@ -103,6 +104,7 @@ fn main() { let mut file = File::options() .write(true) .read(false) + .truncate(true) .open( options .existing diff --git a/crates/i18n-scan/src/minijinja.rs b/crates/i18n-scan/src/minijinja.rs index e7c1cbd4..0ea8a6cb 100644 --- a/crates/i18n-scan/src/minijinja.rs +++ b/crates/i18n-scan/src/minijinja.rs @@ -14,7 +14,7 @@ pub use minijinja::machinery::parse; use minijinja::{ - machinery::ast::{Call, Const, Expr, Macro, Stmt}, + machinery::ast::{Call, Const, Expr, Macro, Spanned, Stmt}, ErrorKind, }; @@ -113,7 +113,11 @@ fn find_in_macro<'a>(context: &mut Context, macro_: &'a Macro<'a>) -> Result<(), Ok(()) } -fn find_in_call<'a>(context: &mut Context, call: &'a Call<'a>) -> Result<(), minijinja::Error> { +fn find_in_call<'a>( + context: &mut Context, + call: &'a Spanned>, +) -> Result<(), minijinja::Error> { + let span = call.span(); if let Expr::Var(var_) = &call.expr { if var_.id == context.func() { let key = call @@ -134,14 +138,18 @@ fn find_in_call<'a>(context: &mut Context, call: &'a Call<'a>) -> Result<(), min } }); - context.record(Key::new( + let key = Key::new( if has_count { crate::key::Kind::Plural } else { crate::key::Kind::Message }, key.to_owned(), - )); + ); + + let key = context.set_key_location(key, span); + + context.record(key); } } diff --git a/crates/i18n/src/sprintf/grammar.pest b/crates/i18n/src/sprintf/grammar.pest index 9ae353c4..4e3bd136 100644 --- a/crates/i18n/src/sprintf/grammar.pest +++ b/crates/i18n/src/sprintf/grammar.pest @@ -80,4 +80,4 @@ text = @{ (!start ~ ANY)+ } start = _{ "%" } number = @{ ASCII_NONZERO_DIGIT ~ ASCII_DIGIT* } -ident = @{ ASCII_ALPHA ~ ASCII_ALPHANUMERIC* } +ident = @{ (ASCII_ALPHA | "_") ~ (ASCII_ALPHANUMERIC | "_")* } diff --git a/crates/i18n/src/translations.rs b/crates/i18n/src/translations.rs index 16f9719c..9b4bd0ee 100644 --- a/crates/i18n/src/translations.rs +++ b/crates/i18n/src/translations.rs @@ -12,10 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{collections::BTreeMap, ops::Deref}; +use std::{ + collections::{BTreeMap, BTreeSet}, + ops::Deref, +}; use icu_plurals::PluralCategory; -use serde::{Deserialize, Serialize}; +use serde::{ + de::{MapAccess, Visitor}, + ser::SerializeMap, + Deserialize, Deserializer, Serialize, Serializer, +}; use crate::sprintf::Message; @@ -30,20 +37,137 @@ fn plural_category_as_str(category: PluralCategory) -> &'static str { } } -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(untagged)] -pub enum TranslationTree { - Message(Message), - Children(BTreeMap), +pub type TranslationTree = Tree; + +#[derive(Debug, Clone, Deserialize, Default)] +pub struct Metadata { + #[serde(skip)] + // We don't want to deserialize it, as we're resetting it every time + // This then generates the `context` field when serializing + pub context_locations: BTreeSet, + pub description: Option, } -impl Default for TranslationTree { - fn default() -> Self { - Self::Children(BTreeMap::new()) +impl Serialize for Metadata { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + let context = self + .context_locations + .iter() + .map(String::as_str) + .collect::>() + .join(", "); + + let mut map = serializer.serialize_map(None)?; + + if !context.is_empty() { + map.serialize_entry("context", &context)?; + } + + if let Some(description) = &self.description { + map.serialize_entry("description", description)?; + } + + map.end() } } -impl TranslationTree { +impl Metadata { + fn add_location(&mut self, location: String) { + self.context_locations.insert(location); + } +} + +#[derive(Debug, Clone, Default)] +pub struct Tree { + inner: BTreeMap, +} + +#[derive(Debug, Clone)] +pub struct Node { + metadata: Option, + value: Value, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(untagged)] +pub enum Value { + Tree(Tree), + Leaf(Message), +} + +impl<'de> Deserialize<'de> for Tree { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct TreeVisitor; + + impl<'de> Visitor<'de> for TreeVisitor { + type Value = Tree; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("map") + } + + fn visit_map(self, mut map: A) -> Result + where + A: MapAccess<'de>, + { + let mut tree: BTreeMap = BTreeMap::new(); + let mut metadata_map: BTreeMap = BTreeMap::new(); + + while let Some(key) = map.next_key::()? { + if let Some(name) = key.strip_prefix('@') { + let metadata = map.next_value::()?; + metadata_map.insert(name.to_owned(), metadata); + } else { + let value = map.next_value::()?; + tree.insert( + key, + Node { + metadata: None, + value, + }, + ); + } + } + + for (key, meta) in metadata_map { + if let Some(node) = tree.get_mut(&key) { + node.metadata = Some(meta); + } + } + + Ok(Tree { inner: tree }) + } + } + + deserializer.deserialize_any(TreeVisitor) + } +} + +impl Serialize for Tree { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + let mut map = serializer.serialize_map(None)?; + + for (key, value) in &self.inner { + map.serialize_entry(key, &value.value)?; + if let Some(meta) = &value.metadata { + map.serialize_entry(&format!("@{key}"), meta)?; + } + } + + map.end() + } +} + +impl Tree { /// Get a message from the tree by key. /// /// Returns `None` if the requested key is not found. @@ -51,7 +175,7 @@ impl TranslationTree { pub fn message(&self, key: &str) -> Option<&Message> { let keys = key.split('.'); let node = self.walk_path(keys)?; - let message = node.as_message()?; + let message = node.value.as_message()?; Some(message) } @@ -65,19 +189,20 @@ impl TranslationTree { let keys = key.split('.'); let node = self.walk_path(keys)?; - let subtree = match node { - TranslationTree::Message(message) => return Some(message), - TranslationTree::Children(tree) => tree, + let subtree = match &node.value { + Value::Leaf(message) => return Some(message), + Value::Tree(tree) => tree, }; - if let Some(node) = subtree.get(plural_category_as_str(category)) { - let message = node.as_message()?; - Some(message) + let node = if let Some(node) = subtree.inner.get(plural_category_as_str(category)) { + node } else { // Fallback to the "other" category - let message = subtree.get("other")?.as_message()?; - Some(message) - } + subtree.inner.get("other")? + }; + + let message = node.value.as_message()?; + Some(message) } #[doc(hidden)] @@ -85,48 +210,100 @@ impl TranslationTree { &mut self, path: I, value: Message, + location: Option, ) -> bool { - let mut path = path.into_iter(); - let Some(next) = path.next() else { - if let TranslationTree::Message(_) = self { - return false; - } - - *self = TranslationTree::Message(value); - return true; + // We're temporarily moving the tree out of the struct to be able to nicely + // iterate on it + let mut fake_root = Node { + metadata: None, + value: Value::Tree(Tree { + inner: std::mem::take(&mut self.inner), + }), }; - match self { - TranslationTree::Message(_) => panic!("cannot set a value on a message node"), - TranslationTree::Children(children) => children - .entry(next.deref().to_owned()) - .or_default() - .set_if_not_defined(path, value), + let mut node = &mut fake_root; + for key in path { + match &mut node.value { + Value::Tree(tree) => { + node = tree.inner.entry(key.deref().to_owned()).or_insert(Node { + metadata: None, + value: Value::Tree(Tree::default()), + }); + } + Value::Leaf(_) => { + panic!() + } + } } + + let replaced = match &node.value { + Value::Tree(tree) => { + assert!( + tree.inner.is_empty(), + "Trying to overwrite a non-empty tree" + ); + + node.value = Value::Leaf(value); + true + } + Value::Leaf(_) => { + // Do not overwrite existing values + false + } + }; + + if let Some(location) = location { + node.metadata + .get_or_insert(Metadata::default()) + .add_location(location); + } + + // Restore the original tree at the end of the function + match fake_root { + Node { + value: Value::Tree(tree), + .. + } => self.inner = tree.inner, + _ => panic!("Tried to replace the root node"), + }; + + replaced } fn walk_path, I: IntoIterator>( &self, path: I, - ) -> Option<&TranslationTree> { - let mut path = path.into_iter(); - let Some(next) = path.next() else { - return Some(self); + ) -> Option<&Node> { + let mut iterator = path.into_iter(); + let Some(next) = iterator.next() else { + return None; }; - match self { - TranslationTree::Message(_) => None, - TranslationTree::Children(tree) => { - let child = tree.get(&*next)?; - child.walk_path(path) - } - } + self.walk_path_inner(next, iterator) } + fn walk_path_inner, I: Iterator>( + &self, + next_key: K, + mut path: I, + ) -> Option<&Node> { + let next = self.inner.get(next_key.deref())?; + + match path.next() { + Some(next_key) => match &next.value { + Value::Tree(tree) => tree.walk_path_inner(next_key, path), + Value::Leaf(_) => None, + }, + None => Some(next), + } + } +} + +impl Value { fn as_message(&self) -> Option<&Message> { match self { - TranslationTree::Message(message) => Some(message), - TranslationTree::Children(_) => None, + Value::Leaf(message) => Some(message), + Value::Tree(_) => None, } } } diff --git a/templates/pages/error.html b/templates/pages/error.html index 219582b9..f2a6ae28 100644 --- a/templates/pages/error.html +++ b/templates/pages/error.html @@ -19,7 +19,7 @@ limitations under the License. {% block content %} diff --git a/translations/en.json b/translations/en.json index 7f6f6279..9791b923 100644 --- a/translations/en.json +++ b/translations/en.json @@ -1,6 +1,26 @@ { "app": { "human_name": "Matrix Authentication Service", - "name": "matrix-authentication-service" + "@human_name": { + "context": "pages/index.html:23:63-82", + "description": "Human readable name of the application" + }, + "name": "matrix-authentication-service", + "@name": { + "context": "app.html:26:14-27, base.html:32:31-44", + "description": "Name of the application" + }, + "technical_description": "OpenID Connect discovery document: %(discovery_url)s", + "@technical_description": { + "context": "pages/index.html:25:11-70", + "description": "Introduction text displayed on the home page" + } + }, + "error": { + "unexpected": "Unexpected error", + "@unexpected": { + "context": "pages/error.html:22:41-62", + "description": "Error message displayed when an unexpected error occurs" + } } } \ No newline at end of file