Skip to content

Conversation

@BLumia
Copy link
Member

@BLumia BLumia commented Jun 24, 2025

This is mainly for gettext support.

Log:

Summary by Sourcery

Add support for <lang> placeholder in pattern paths to resolve language-specific files by filename or directory structure

New Features:

  • Allow <lang> in target filenames to match and capture language codes from filenames in a directory
  • Allow <lang> in path segments to match language code subdirectories and locate corresponding files

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sourcery-ai
Copy link

sourcery-ai bot commented Jun 24, 2025

Reviewer's Guide

The PR refactors the target file discovery to branch on whether the <lang> placeholder appears in the file name or in the path. If it’s in the file name, it uses the existing regex-based filter; otherwise, it parses path components to locate language-named subdirectories, validates them, and assembles the target paths accordingly.

Class diagram for Filter::find_target_files method update

classDiagram
    class Filter {
        +find_target_files(target_pattern_path: &Path) -> Result<Vec<(String, PathBuf)>>
    }
    Filter : +find_target_files now supports <lang> in path
    Filter : +find_target_files branches on <lang> in file name vs path
Loading

Flow diagram for target file discovery with in file name vs path

flowchart TD
    A[Start: target_filename_pattern] --> B{Does target_filename_pattern contain <lang>?}
    B -- Yes --> C[Use regex filter on file names in parent directory]
    C --> D[Collect matched files with language code]
    B -- No --> E[Parse path components to find <lang>]
    E --> F{Is <lang> found in path?}
    F -- No --> G[Return error: Missing <lang> in pattern]
    F -- Yes --> H[Iterate subdirectories in parent_dir]
    H --> I[Validate subdirectory as language code]
    I --> J[Check if file exists in subdirectory]
    J --> K[Collect matched files with language code]
    D & K --> L[Return matched files]
Loading

File-Level Changes

Change Details Files
Branch on <lang> placeholder location to support both filename- and path-based patterns
  • Add an if target_filename_pattern.contains("<lang>") guard around the original filename-based logic
  • Move create_filter_pattern call into the filename branch
  • Indent and return early in the filename branch after collecting matches
  • Introduce an else branch for path-based <lang> handling
src/transifex/yaml_file.rs
Implement directory scanning for path-based <lang> patterns
  • Split target_pattern_path into parent_dir (up to <lang>) and remain_path (after <lang>)
  • Read parent_dir and iterate over entries as potential language folders
  • Validate folder names against [a-z_A-Z]{2,6} regex
  • For each valid folder, join it with remain_path, check if the file exists, and collect matches
src/transifex/yaml_file.rs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @BLumia - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments

### Comment 1
<location> `src/transifex/yaml_file.rs:132` </location>
<code_context>
+                    parent_dir.push(component);
+                }
+            };
+            if remain_path == None {
+                return Err(std::io::Error::new(std::io::ErrorKind::Other, "Missing <lang> inside the pattern."));
+            }
</code_context>

<issue_to_address>
Consider using `is_none()` instead of `== None` for Option checks.

This change aligns with Rust idioms and prevents clippy warnings.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
            if remain_path == None {
                return Err(std::io::Error::new(std::io::ErrorKind::Other, "Missing <lang> inside the pattern."));
            }
=======
            if remain_path.is_none() {
                return Err(std::io::Error::new(std::io::ErrorKind::Other, "Missing <lang> inside the pattern."));
            }
>>>>>>> REPLACE

</suggested_fix>

### Comment 2
<location> `src/transifex/yaml_file.rs:145` </location>
<code_context>
+                    let Some(language_folder) = language_folder.to_str() else {
+                        continue;
+                    };
+                    if !regex::Regex::new(r"[a-z_A-Z]{2,6}").unwrap().is_match(language_folder) {
+                        continue;
+                    }
</code_context>

<issue_to_address>
Regex is compiled on every iteration, which is inefficient.

Compile the regex once before the loop and reuse it to improve performance.
</issue_to_address>

### Comment 3
<location> `src/transifex/yaml_file.rs:148` </location>
<code_context>
+                    if !regex::Regex::new(r"[a-z_A-Z]{2,6}").unwrap().is_match(language_folder) {
+                        continue;
+                    }
+                    let matched_file = language_folder_dir.join(remain_path.clone().unwrap());
+                    if !matched_file.is_file() {
+                        continue;
</code_context>

<issue_to_address>
Repeatedly calling `clone().unwrap()` on `remain_path` can be avoided.

Unwrap `remain_path` once before the loop and reuse the value to avoid unnecessary cloning and unwrapping in each iteration.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

This is mainly for gettext support.

Log:
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 错误处理:在处理文件路径和读取目录时,应该检查read_dirjoin等操作是否可能失败,并相应地处理错误。例如,read_dirjoin操作应该被包裹在matchif let语句中,以处理可能的错误。

  2. 正则表达式:在创建language_code_regex时,使用了unwrap方法,这可能会导致程序在正则表达式编译失败时崩溃。应该使用Result类型来处理正则表达式的创建,并在失败时返回错误。

  3. 代码重复:在处理target_fileslanguage_folders时,代码结构相似,可以考虑提取公共逻辑到一个单独的函数中,以减少代码重复。

  4. 变量命名:变量remain_pathparent_dir的命名不够清晰,建议使用更具描述性的名称,如base_dirremaining_path

  5. 性能优化:在循环中,对每个文件名和文件夹名进行正则表达式匹配可能会影响性能。如果可能,应该尽量减少正则表达式的使用频率。

  6. 安全性:代码中使用了unwrap方法,这可能会导致程序在遇到错误时崩溃。应该使用matchif let语句来处理可能的错误,并返回适当的错误信息。

  7. 代码注释:代码中缺少足够的注释,特别是对于复杂的逻辑和错误处理,应该添加注释来解释代码的目的和逻辑。

  8. 路径处理:在处理路径时,应该使用PathPathBufjoin方法来确保路径的正确性,而不是手动拼接字符串。

  9. 资源管理:在处理文件和目录时,应该使用Result类型来处理可能的错误,并确保资源被正确释放。例如,在read_dirjoin操作后,应该检查Result是否成功,并在失败时释放已分配的资源。

  10. 代码风格:代码风格不一致,例如,有些地方使用了空格缩进,有些地方使用了制表符缩进。建议统一使用一种缩进方式,并遵循Rust的代码风格指南。

@BLumia BLumia merged commit 71daad5 into master Jun 24, 2025
9 checks passed
@BLumia BLumia deleted the lang-in-path branch June 24, 2025 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants