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

Export env language var matching document state #2064

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
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
2 changes: 2 additions & 0 deletions core/settings.lua
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ function settings:_init ()
]]):format(language, language, language, language))
end
fluent:set_locale(language)
os.setlocale(language)
SU.setenv("LANG", language)
Copy link
Member

@Omikhleia Omikhleia Jun 24, 2024

Choose a reason for hiding this comment

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

While this could work for some language, the correct approach might be trickier:

  • LANGexpect a Unix locale (eg. de_DE)
  • Our current document.language is only a two letter code (eg. de), which may be under-resolved
  • If we move our document.language at some point to possibly fit either BCP47 or ICU locales, it still likely wouldn't be a valid Unix locale (e.g. consider e.g. BCP47 code sr-Latn-RS, ICU locale sr_Latn_RS, Unix locale sr_RS@latin)

So we might have to consider conversion utilities (BCP47 -> ICU locale -> Unix locale)

Copy link
Member

Choose a reason for hiding this comment

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

While there are one-to-one canonicalization rules between BCP47 languages and ICU locales (which our justenoughicu wrapper even implements), I can't find any clear source for mapping to Unix/Posix locales (setlocale) as expected by LANG. So it might be a a hornest's nest.

(BTW, Why would we want it? For Lua os.date etc. to be localized? I'm not sure there's a good use case for it)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this whole maneuver is dependent on us starting to use proper locales (BCP47 or whatever) internally and on adding a mapping to the setenv() function that converts whatever than scheme is to a Unix locale.

I'm already using this is production with a small hack to map the languages I use. I have documents where one class is used to render inputs in multiple languages, and for example the inprint page has date information. This is rendered with the system date command, but in only outputs the correct localized string if the locale is set. This already works if I supply the right mapping. I also have other tools besides date that output content that gets ingested during the SILE render process (for figures, music, etc.) and is sensitive to the locale.

The real question is whether there is ever a case that setting the LANG will produce undesired results. For it to work the system tools need to have support for the expected locales compiled using locale-gen or similar, but would the user ever not expect it to work this way? I couldn't think of any time this would be objectively wrong.

Copy link
Member

Choose a reason for hiding this comment

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

Aside note: for rendering a properly localized date (say, today as "10 Eylül 2024 Salı" / "вторник, 10 сентября 2024 г." / "mardi 10 septembre 2024"), rather than using Lua's os.date and expect the host has some appropriate Unix locale.... we might wrap the ICU methods in our "justenoughicu" library: https://unicode-org.github.io/icu/userguide/format_parse/datetime/examples.html#c-1 --- so one can feed an epoch time, however obtained, to ICU for proper rendering according to the locale, and the expected format (full, long, medium, etc.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, except that instead of having our own custom "just enough" wrapper for ICU we're going to have access to the full ICU API surface via Rust crates (and of course can expose whatever bits of it we want on the Lua side, which is basically what we have now in justenough... except we won't need the C wrapper.

end,
help = "Locale for localized language support",
})
Expand Down
28 changes: 28 additions & 0 deletions core/utilities/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,34 @@ function utilities.rationWidth (target, width, ratio)
return target
end

--- System
-- @section system

local function which_setenv ()
if type(_G._rusty_funcs) == "table" then
return _G._rusty_funcs.setenv
elseif pcall(require, "posix") then
return require("posix").stdlib.setenv
elseif pcall(require, "winapi") then
return require("winapi").setenv
else
return function (key, _)
utilities.warn(([[Failed to set environment variable '%s' using legacy Lua based CLI

Please use the Rust based CLI. Meanwhile expect anything depending on
this variable not to work. Alternatively installing the luaposix or
winapi LuaRocks modules will enable this from the Lua side.

]]):format(key))
end
end
end

--- Set environment variable to take effect when executing subprocesses
-- @tparam string key Name of environment variable, typically in all caps.
-- @tparam string value Value to set.
utilities.setenv = which_setenv()

--- Text handling
-- @section text

Expand Down
6 changes: 6 additions & 0 deletions sile.rockspec.in
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ dependencies = {
"vstruct == 2.1.1-1",
}

-- Optional deps to enable the legacy Lua CLI to have features from the Rust one
--[[
table.insert(dependencies, "luaposix == 36.2.1-1") -- for Unix
table.insert(dependencies, "winapi == 1.4.2-1") -- for Windows
]]

build = {
type = "builtin",
modules = {},
Expand Down
7 changes: 7 additions & 0 deletions spec/languages_spec.lua
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
SILE = require("core.sile")

describe("Language module", function ()

it("should set env locale", function ()
SILE.call("language", { main = "tr" })
local syslang = os.getenv("LANG")
assert.is.equal("tr", syslang)
end)

describe("Norwegian", function ()
local hyphenate = SILE.showHyphenationPoints

Expand Down
12 changes: 11 additions & 1 deletion spec/utilities_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,24 @@ describe("SILE.utilities", function ()
end)
end)

describe("utf8_to_utf16be_hexencoded ", function ()
describe("utf8_to_utf16be_hexencoded", function ()
it("should hex encode input", function ()
local str = "foo"
local out = "feff0066006f006f"
assert.is.equal(out, SU.utf8_to_utf16be_hexencoded(str))
end)
end)

describe("setenv", function ()
it("should effectively set variables before executing system commands", function ()
local before = os.getenv("FOO")
assert.is.equal(type(before), "nil")
SU.setenv("FOO", "bar")
local after = os.getenv("FOO")
assert.is.equal(after, "bar")
end)
end)

describe("formatNumber", function ()
local icu = require("justenoughicu")
local icu73plus = tostring(icu.version()) >= "73.0"
Expand Down
13 changes: 13 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub fn start_luavm() -> crate::Result<Lua> {
#[cfg(feature = "static")]
crate::embed::inject_embedded_loader(&lua);
inject_paths(&lua);
inject_rusty_funcs(&lua);
load_sile(&lua);
inject_version(&lua);
Ok(lua)
Expand Down Expand Up @@ -50,6 +51,18 @@ pub fn inject_paths(lua: &Lua) {
}
}

pub fn inject_rusty_funcs(lua: &Lua) {
let rusty_funcs: LuaTable = lua.create_table().unwrap();
let setenv: LuaFunction = lua
.create_function(|_, (key, value): (String, String)| {
env::set_var(key, value);
Ok(())
})
.unwrap();
rusty_funcs.set("setenv", setenv).unwrap();
lua.globals().set("_rusty_funcs", rusty_funcs).unwrap();
}

pub fn inject_version(lua: &Lua) {
let sile: LuaTable = lua.globals().get("SILE").unwrap();
let mut full_version: String = sile.get("full_version").unwrap();
Expand Down
Loading