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

Add methods to iterate exemplar characters #6079

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ffi/capi/bindings/js/ExemplarCharacters.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 25 additions & 0 deletions ffi/capi/bindings/js/ExemplarCharacters.mjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions ffi/capi/bindings/js/StringIterator.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

57 changes: 57 additions & 0 deletions ffi/capi/bindings/js/StringIterator.mjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions ffi/capi/bindings/js/index.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions ffi/capi/bindings/js/index.mjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 23 additions & 0 deletions ffi/capi/src/exemplar_chars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#[diplomat::attr(auto, namespace = "icu4x")]
pub mod ffi {
use alloc::boxed::Box;
use crate::{properties_iter::ffi::CodePointRangeIterator, string_iter::ffi::StringIterator};

#[cfg(feature = "buffer_provider")]
use crate::provider::ffi::DataProvider;
Expand Down Expand Up @@ -47,6 +48,28 @@ pub mod ffi {
self.0.as_borrowed().contains32(cp)
}

/// Get an iterator of all the code point ranges in the current exemplar character set.
#[diplomat::rust_link(
icu::collections::codepointinvliststringlist::CodePointInversionListAndStringList::code_points,
FnInStruct
)]
pub fn code_point_ranges<'a>(&'a self) -> Box<CodePointRangeIterator<'a>> {
let ranges = self.0.as_borrowed().code_points().iter_ranges().collect::<Vec<_>>();

Box::new(CodePointRangeIterator(Box::new(ranges.into_iter())))
}

/// Get an iterator of all the code point ranges in the current exemplar character set.
#[diplomat::rust_link(
icu::collections::codepointinvliststringlist::CodePointInversionListAndStringList::strings,
FnInStruct
)]
pub fn strings(&self) -> Box<StringIterator> {
let strings = self.0.as_borrowed().strings().iter().map(|s| s.to_string()).collect::<Vec<_>>();
Copy link
Member

@Manishearth Manishearth Feb 7, 2025

Choose a reason for hiding this comment

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

issue: these two should not collect, Diplomat can return borrows. This should return a Box<ExemplarCharactersStringIterator<'a>>

Ideally there's no dynamic dispatch, if we need concrete iterator types in ICU4X (where we have `impl Trait) we should add them.


Box::new(StringIterator(Box::new(strings.into_iter())))
}

/// Create an [`ExemplarCharacters`] for the "main" set of exemplar characters for a given locale, using compiled data.
#[diplomat::rust_link(
icu::locale::exemplar_chars::ExemplarCharacters::try_new_main,
Expand Down
2 changes: 2 additions & 0 deletions ffi/capi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ pub mod properties_sets;
pub mod properties_unisets;
#[cfg(feature = "properties")]
pub mod script;
#[cfg(feature = "properties")]
pub mod string_iter;
#[cfg(feature = "segmenter")]
pub mod segmenter_grapheme;
#[cfg(feature = "segmenter")]
Expand Down
25 changes: 25 additions & 0 deletions ffi/capi/src/string_iter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// This file is part of ICU4X. For terms of use, please see the file
// called LICENSE at the top level of the ICU4X source tree
// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).

#[diplomat::bridge]
#[diplomat::abi_rename = "icu4x_{0}_mv1"]
#[diplomat::attr(auto, namespace = "icu4x")]
pub mod ffi {
use core::fmt::Write;
use alloc::boxed::Box;

/// An iterator over strings
#[diplomat::opaque]
pub struct StringIterator(
pub Box<dyn Iterator<Item = String>>,
Copy link
Member

Choose a reason for hiding this comment

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

issue: this should reference a concrete type, and borrow from it

);

impl StringIterator {
/// Advance the iterator by one and return the next string, terminated with a null byte.
/// If there are no more strings to be iterated, an empty string is returned.
pub fn next(&mut self, write: &mut DiplomatWrite) {
Copy link
Member

Choose a reason for hiding this comment

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

issue: Diplomat supports iterators, I suggest tacking on #[diplomat::attr(auto, iterator)] and having this return an Option<()>.

We should probably make sure all other iterable APIs in icu_capi use iterators too.

let _ = write.write_str(&self.0.next().map(|mut s| { s.push('\0'); s }).unwrap_or_default());
}
}
}
58 changes: 57 additions & 1 deletion tutorials/npm/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@
<li class="nav-item" role="presentation">
<button class="nav-link" data-bs-toggle="pill" data-bs-target="#pills-seg" type="button" role="tab" aria-controls="pills-seg" aria-selected="false">Segmenter</button>
</li>
<li class="nav-item" role="presentation">
<button class="nav-link" data-bs-toggle="pill" data-bs-target="#pills-ech" type="button" role="tab" aria-controls="pills-ech" aria-selected="false">Exemplar Characters</button>
</li>
</ul>
</div>

Expand Down Expand Up @@ -235,6 +238,59 @@
</div>
</div>
</div>
<div class="tab-pane" id="pills-ech" role="tabpanel" aria-labelledby="pills-ech-tab" tabindex="0">
<div class="vstack gap-2">
<label>
Locale
<div class="btn-toolbar" role="toolbar" aria-label="Fixed Decimal Formatting options">
<div class="btn-group" role="group" aria-label="Fixed Decimal Formatting Locale">
<input type="radio" class="btn-check" name="ech-locale" id="ech-locale-en" value="en" checked>
<label class="btn btn-outline-primary" for="ech-locale-en">en</label>

<input type="radio" class="btn-check" name="ech-locale" id="ech-locale-bn" value="bn">
<label class="btn btn-outline-primary" for="ech-locale-bn">bn</label>

<div class="input-group">
<input type="radio" class="btn-check" name="ech-locale" id="ech-locale-other" value="other">
<label class="btn btn-outline-primary" for="ech-locale-other">other</label>
<input type="text" id="ech-locale-other-input" class="form-control" placeholder="Locale ID">
</div>
</div>
</div>
</label>

<div class="card">
<div class="card-header">Main</div>
<div class="card-body">
<p id="ech-output-main" class="card-text"></p>
</div>
</div>
<div class="card">
<div class="card-header">Auxiliary</div>
<div class="card-body">
<p id="ech-output-auxiliary" class="card-text"></p>
</div>
</div>
<div class="card">
<div class="card-header">Punctuation</div>
<div class="card-body">
<p id="ech-output-punctuation" class="card-text"></p>
</div>
</div>
<div class="card">
<div class="card-header">Numbers</div>
<div class="card-body">
<p id="ech-output-numbers" class="card-text"></p>
</div>
</div>
<div class="card">
<div class="card-header">Index</div>
<div class="card-body">
<p id="ech-output-index" class="card-text"></p>
</div>
</div>
</div>
</div>
</div>
</div>
<div id="bigspinner">
Expand Down Expand Up @@ -287,4 +343,4 @@
}
</style>
</body>
</html>
</html>
4 changes: 3 additions & 1 deletion tutorials/npm/src/ts/app.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as fdf from './fixed-decimal';
import * as dtf from './date-time';
import * as seg from './segmenter';
import * as ech from './exemplar-characters';

import 'bootstrap/js/dist/tab';
import 'bootstrap/js/dist/dropdown';
Expand All @@ -10,5 +11,6 @@ import 'bootstrap/js/dist/collapse';
fdf.setup();
dtf.setup();
seg.setup();
ech.setup();
(document.querySelector("#bigspinner") as HTMLElement).style.display = "none";
})()
})()
Loading
Loading