Release/v2.0.4 by tamada · Pull Request #41 · tamada/sibling

tamada · 2026-01-25T22:47:40Z

No description provided.

…ing. upgrading with enhanced clippy check (cargo clippy -- -W clippy::pedantic)

…se/v2.0.4

+    runs-on: ubuntu-latest
+    needs: publish
+    outputs:
+      appname: ${{ needs.publish.outputs.appname }}
+      tag: ${{ needs.publish.outputs.tag }}
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Authenticate with custom registry
+        id: auth
+        uses: rust-lang/crates-io-auth-action@v1
+
+      - name: publish
+        env:
+          CARGO_REGISTRY_TOKEN: ${{ steps.auth.outputs.token }}
+        run:
+          cargo publish --package sibling


In general, the fix is to add an explicit permissions block so the GITHUB_TOKEN is scoped to the least privileges required. You can add it at the top level (applying to all jobs) or per‑job. Since all jobs here are part of the same release/publish pipeline and at least the setup, documents and publish jobs clearly need to write releases (which uses contents: write), the simplest safe change is to define a workflow‑level permissions block with contents: write. That both satisfies CodeQL (permissions are now explicit) and matches the actual needs of the workflow.

The single best way to fix this without changing behavior is:

Add a root‑level permissions block right after the name: Publish (or just after on:) in .github/workflows/publish.yaml.

Set contents: write so the jobs that create and upload releases continue to function.

Optionally, you could further restrict other scopes (e.g., leave all others at their implicit none), but we won’t guess additional scopes: contents: write is clearly needed and sufficient for interacting with releases.

Concretely, edit .github/workflows/publish.yaml near the top, add:

permissions: contents: write

leaving all jobs and steps unchanged.

Copilot

Pull request overview

This pull request bumps the version from 2.0.3 to 2.0.4 with significant code improvements and API changes. The changes include refactoring from usize to i32 for directory counts, renaming internal fields, adding #[must_use] attributes, introducing a new JSON output format, and improving code quality throughout.

Changes:

Version bumped to 2.0.4 across all package files and documentation
API breaking changes: Dirs::len() return type changed from usize to i32, and NexterFactory::build() now takes a reference parameter
New JSON output format added alongside existing CSV and list formats
Code quality improvements including better error handling, consistent use of references, and simplified control flow

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 17 comments.

Show a summary per file

File	Description
Cargo.toml	Version bump to 2.0.4
README.md	Version badge updated to 2.0.4
docs/config.toml	Documentation version updated to 2.0.4
lib/README.md	Enhanced CLI reference with GitHub link
lib/src/lib.rs	Major refactoring: field renamed (dirs→entries), return type changed (usize→i32), improved error handling, added #[must_use] attributes, updated tests
cli/src/cli.rs	Replaced separate --csv and --list flags with unified --format enum
cli/src/printer.rs	Added JSON format support, simplified function signatures (removed Result wrapper)
cli/src/main.rs	Updated to use reference parameters for API calls
cli/src/init.rs	Simplified shell matching with pattern matching
cli/src/gencomp.rs	Refactored to use references instead of owned values
.github/workflows/publish.yaml	Added crates.io publishing workflow

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-25T22:53:46Z

+    entries: Vec<PathBuf>,
    /// The parent directory that contains all the sibling directories.
    parent: PathBuf,
    /// The index of the current directory in the `dirs` vector.


The comment still refers to dirs vector but the field was renamed to entries. The comment should be updated to say "The index of the current directory in the entries vector."

Copilot · 2026-01-25T22:53:46Z

+    /// 
+    /// ## Panics
+    /// 
+    /// This function will panic if the number of directories exceeds `i32::MAX`.
+    #[must_use]
+    pub fn len(&self) -> i32 {
+        i32::try_from(self.entries.len()).unwrap()


Changing the return type from usize to i32 is a breaking API change that could affect external consumers of this library. While the reasoning may be valid (to align with the internal use of i32 for step calculations), this change should be carefully considered. It reduces the maximum supported number of directories from usize::MAX to i32::MAX (approximately 2.1 billion). Consider whether this limitation is acceptable, or if there's a better way to handle the internal arithmetic without changing the public API.

Suggested change

///

/// ## Panics

///

/// This function will panic if the number of directories exceeds `i32::MAX`.

#[must_use]

pub fn len(&self) -> i32 {

i32::try_from(self.entries.len()).unwrap()

#[must_use]

pub fn len(&self) -> usize {

self.entries.len()

Copilot · 2026-01-25T22:53:46Z

    /// Get the next directory using the given [`Nexter`] and step.
+    /// 
+    /// ## Panics
+    /// 


There is trailing whitespace on this line. Consider removing it for consistency with code style guidelines.

Copilot · 2026-01-25T22:53:47Z

    let mut result = vec![];
    let current = dirs.current();
-    let next_index = next.clone().map(|n| n.index() as i32).unwrap_or(-1);
+    let next_index = next.map_or(-1, |n| i32::try_from(n.index()).unwrap());


The conversion from usize to i32 can panic if the index exceeds i32::MAX. While unlikely in practice, if the system has extremely large directory lists, this could cause a panic. Consider handling this error gracefully or documenting the assumption that directory indices will never exceed i32::MAX.

Copilot · 2026-01-25T22:53:47Z

    /// Build a [`Nexter`] instance based on the given [`NexterType`].
-    pub fn build(nexter_type: NexterType) -> Box<dyn Nexter> {
+    #[must_use]
+    pub fn build(nexter_type: &NexterType) -> Box<dyn Nexter> {


Changing the parameter from NexterType to &NexterType is a breaking API change. While it's a more idiomatic Rust pattern (especially since NexterType implements Clone and this avoids unnecessary cloning), external consumers will need to update their code. Ensure this breaking change is documented in the release notes.

Suggested change

pub fn build(nexter_type: &NexterType) -> Box<dyn Nexter> {

pub fn build(nexter_type: NexterType) -> Box<dyn Nexter> {

Copilot · 2026-01-25T22:53:49Z

+    let length = dirs.len();
    log::trace!(
        "next_impl(step={step}, current={}, next={next})",
        dirs.current
    );
-    if next < 0 || next >= dirs.dirs.len() as i32 {
+    if next < 0 || next >= dirs.len() {
        log::warn!(
            "next_impl: out of range (next={next}, len={})",
-            dirs.dirs.len()
+            dirs.len()


The variable length is assigned on line 465 but only used once on line 478. Lines 470 and 473 call dirs.len() directly instead of using the cached value. For consistency and to avoid unnecessary repeated function calls, consider using length in all three places, or remove the variable if caching is not needed.

Copilot · 2026-01-25T22:53:49Z

+        let next = dirs.len() - 1;
+        Some(Dir::new_of_last_item(dirs, usize::try_from(next).unwrap()))


If the directory list is empty (len() returns 0), this code will compute 0 - 1 = -1 and then call usize::try_from(-1).unwrap() which will panic. Consider checking if the directory list is empty first and returning None in that case, or handle the empty case explicitly.

Copilot · 2026-01-25T22:53:49Z


 fn next_impl(dirs: &Dirs, step: i32) -> Option<Dir<'_>> {
-    let next = dirs.current as i32 + step;
+    let next = i32::try_from(dirs.current).unwrap() + step;


If dirs.current exceeds i32::MAX, this conversion will panic. Since dirs.current is a usize, it could theoretically hold values larger than i32::MAX. While the panic documentation is added for len(), similar consideration should be given here. Consider documenting this constraint or handling the conversion error gracefully.

Suggested change

let next = i32::try_from(dirs.current).unwrap() + step;

let current_i32 = match i32::try_from(dirs.current) {

Ok(v) => v,

Err(_) => {

log::error!(

"next_impl: current index {} exceeds i32::MAX and cannot be converted",

dirs.current

);

return None;

}

};

let next = current_i32 + step;

Copilot · 2026-01-25T22:53:49Z

+    /// 
+    /// ## Panics
+    /// 


There is trailing whitespace on this line. Consider removing it for consistency with code style guidelines.

Suggested change

///

/// ## Panics

///

///

/// ## Panics

///

Copilot · 2026-01-25T22:53:50Z

    }

    /// Get the next directory using the given [`Nexter`] and step.
+    /// 


There is trailing whitespace on this line. Consider removing it for consistency with code style guidelines.

…ript

…d methods

tamada and others added 5 commits January 23, 2026 21:18

feat: enhance CLI options with format support and refactor path handl…

30f1e6d

…ing. upgrading with enhanced clippy check (cargo clippy -- -W clippy::pedantic)

update version to 2.0.4, ready to publish v2.0.4

4b79451

feat: add publish_to_crates_io job for automated package publishing

5ed636f

docs: clarify CLI command usage in README

932ca55

Merge branch 'release/v2.0.4' of github.com:tamada/sibling into relea…

4b9401b

…se/v2.0.4

tamada added this to the v2.0.0 milestone Jan 25, 2026

tamada requested a review from Copilot January 25, 2026 22:47

tamada self-assigned this Jan 25, 2026

tamada added the enhancement New feature or request label Jan 25, 2026

Copilot started reviewing on behalf of tamada January 25, 2026 22:47 View session

github-advanced-security AI found potential problems Jan 25, 2026

View reviewed changes

Copilot AI reviewed Jan 25, 2026

View reviewed changes

tamada added 5 commits January 26, 2026 08:15

fix: correct version badge formatting in README and update version sc…

79dcb2d

…ript

refactor: rename 'dirs' to 'siblings' in Dir struct and update relate…

9818862

…d methods

refactor: remove unnecessary references in NexterFactory build calls

a91d6a7

refactor: rename NexterFactory build method to create for consistency

7abb746

refactor: simplify return statements in Nexter implementations

ae7190d

tamada merged commit 4c6885c into main Jan 26, 2026
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Release/v2.0.4#41

Release/v2.0.4#41
tamada merged 10 commits into
mainfrom
release/v2.0.4

tamada commented Jan 25, 2026

Uh oh!

Check warning

Copilot Autofix

Copilot AI left a comment

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

@@ -6,6 +6,9 @@
                   - main
                 types: [closed]
+            permissions:
+              contents: write
             jobs:
               setup:
                 runs-on: ubuntu-latest

	pub fn build(nexter_type: &NexterType) -> Box<dyn Nexter> {
	pub fn build(nexter_type: NexterType) -> Box<dyn Nexter> {

		let next = dirs.len() - 1;
		Some(Dir::new_of_last_item(dirs, usize::try_from(next).unwrap()))

-    let next = i32::try_from(dirs.current).unwrap() + step;
+    let current_i32 = match i32::try_from(dirs.current) {
+        Ok(v) => v,
+        Err(_) => {
+            log::error!(
+                "next_impl: current index {} exceeds i32::MAX and cannot be converted",
+                dirs.current
+            );
+            return None;
+        }
+    };
+    let next = current_i32 + step;

Conversation

tamada commented Jan 25, 2026

Uh oh!

Check warning

Copilot Autofix

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants