Skip to content

xgettext: Rough draft #359

New issue

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

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

Already on GitHub? # to your account

Closed
wants to merge 5 commits into from
Closed

xgettext: Rough draft #359

wants to merge 5 commits into from

Conversation

fox0
Copy link
Contributor

@fox0 fox0 commented Oct 24, 2024

No description provided.

@jgarzik jgarzik added the enhancement New feature or request label Apr 2, 2025
@jgarzik jgarzik requested a review from Copilot April 2, 2025 21:26
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces a rough draft for the xgettext tool in Rust, aimed at extracting gettext strings from source files for internationalization support in the posixutils-rs project.

  • Adds the main implementation in i18n/xgettext.rs for processing Rust source files and a placeholder for C files.
  • Integrates several tests under i18n/tests/xgettext for various gettext functions (pgettext, npgettext, ngettext, and clap integration).
  • Updates Cargo.toml to add necessary parsing dependencies and registers the xgettext binary.

Reviewed Changes

Copilot reviewed 19 out of 26 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
i18n/xgettext.rs New implementation for xgettext functionality with placeholder for C file support.
i18n/tests/xgettext/*.rs Several tests verifying different gettext-related functionalities.
i18n/tests/xgettext/mod.rs Integration tests for xgettext command generating pot files.
i18n/tests/i18n-tests.rs Added module inclusion for xgettext tests.
i18n/iconv.rs Minor update to the help message string for iconv.
i18n/Cargo.toml Added new dependencies and configured a new binary target for xgettext.
Files not reviewed (7)
  • Makefile: Language not supported
  • i18n/tests/xgettext/test_clap.pot: Language not supported
  • i18n/tests/xgettext/test_gettext.pot: Language not supported
  • i18n/tests/xgettext/test_gettext_no_lines.pot: Language not supported
  • i18n/tests/xgettext/test_ngettext.pot: Language not supported
  • i18n/tests/xgettext/test_npgettext.pot: Language not supported
  • i18n/tests/xgettext/test_pgettext.pot: Language not supported

}

pub fn process_c_file(&mut self, _path: PathBuf) -> std::io::Result<()> {
todo!();
Copy link
Preview

Copilot AI Apr 2, 2025

Choose a reason for hiding this comment

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

The process_c_file function is unimplemented (using todo!()). Consider handling unsupported file types gracefully or returning a proper error message.

Suggested change
todo!();
Err(std::io::Error::new(
std::io::ErrorKind::Unsupported,
"Processing C files is not supported",
))

Copilot uses AI. Check for mistakes.

let path = path.into_os_string().into_string().unwrap();
walker.process_rust_file(content, path)?;
}
_ => todo!(),
Copy link
Preview

Copilot AI Apr 2, 2025

Choose a reason for hiding this comment

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

Using todo!() for unsupported file extensions may lead to a runtime panic. Replace it with clear error handling and message for unsupported file types.

Suggested change
_ => todo!(),
_ => {
eprintln!("xgettext: {}", gettext("unsupported file type"));
exit(1);
},

Copilot uses AI. Check for mistakes.

Comment on lines +274 to +276
// TODO remove this
args.keyword_spec.push("gettext".into());

Copy link
Preview

Copilot AI Apr 2, 2025

Choose a reason for hiding this comment

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

A temporary hack is present where 'gettext' is pushed to the keyword_spec. Remove or properly document this workaround before production use to prevent unintended behavior.

Suggested change
// TODO remove this
args.keyword_spec.push("gettext".into());

Copilot uses AI. Check for mistakes.

@fox0
Copy link
Contributor Author

fox0 commented Apr 3, 2025

#403

@fox0 fox0 closed this Apr 3, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants