Skip to content

fixed: only submodule commit failed#1

Open
jeffinity wants to merge 2 commits intoImSingee:masterfrom
jeffinity:hotfix/only-submodule-commit
Open

fixed: only submodule commit failed#1
jeffinity wants to merge 2 commits intoImSingee:masterfrom
jeffinity:hotfix/only-submodule-commit

Conversation

@jeffinity
Copy link
Copy Markdown

@jeffinity jeffinity commented May 26, 2025

image

Summary by CodeRabbit

Release Notes

  • New Features

    • Added --status option to control which Git-selected files are processed: unstaged, tracked, untracked, or all (default: staged).
    • Expanded file selection flexibility to work with different Git file states, not just staged files.
  • Documentation

    • Updated README with new usage examples and clarified file selection and stash management behavior.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 21, 2026

📝 Walkthrough

Walkthrough

The lint-staged extension was updated to support selecting files by different Git statuses via a new --status option, replacing the "staged files only" model. File selection now delegates to mode-specific helpers, and index management is conditionally applied. A new task flow-breaking mechanism was added to the task system.

Changes

Cohort / File(s) Summary
Documentation & Error Naming
README.md, internal/ext/lint-staged/errors.go
Updated documentation to describe "git-selected files" and added --status usage examples; renamed ErrGetStagedFiles to ErrGetSelectedFiles.
Selection Mode Logic
internal/ext/lint-staged/selection.go, internal/ext/lint-staged/selection_test.go
Introduced new SelectionMode type with five constants (staged, unstaged, untracked, tracked, all) and added methods on Options to derive effective mode, validate inputs, determine index usage, and generate selection labels/reasons; includes comprehensive test coverage.
File Selection Refactoring
internal/ext/lint-staged/git.go
Refactored file selection from single-mode getStagedFiles(options) to getSelectedFiles(options) supporting multiple modes via new helpers; added diff-filter normalization and split staged/unstaged into separate functions; introduced uniqueStrings deduplication utility.
Index Management Control
internal/ext/lint-staged/gitWorkflow.go
Added manageIndex and breakReason fields to gitWorkflow, making prepare() and applyModifications() conditional on manageIndex; added prepareOK() to signal workflow state.
CLI & Option Validation
internal/ext/lint-staged/main.go, internal/ext/lint-staged/messages.go
Added --status option to CLI; introduced validation step calling ValidateSelectionMode(); updated skippingBackup() to derive reason from Options instead of raw diff string.
Runtime Integration
internal/ext/lint-staged/run.go
Switched file collection from getStagedFiles to getSelectedFiles; conditioned backup stash on options.UsesIndex(); set gitWorkflow.manageIndex and added BreakFlow hook to first task; updated messaging to "selected files".
Task Flow Control
internal/lib/tl/task.go, internal/lib/tl/list.go
Added optional BreakFlow callback to Task struct; updated TaskList.start to support breaking remaining task flow based on BreakFlow result.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant main.go as main
    participant run.go as run
    participant git.go as git
    participant Task as Task Flow
    
    User->>main: Invoke with --status unstaged
    main->>main: Validate --status option<br/>ValidateSelectionMode()
    main->>run: runAll(options)
    
    run->>run: Set gitWorkflow.manageIndex<br/>= UsesIndex() [false for unstaged]
    
    run->>Task: Task: Prepare lint-staged
    Task->>run: BreakFlow hook (prepareOK)<br/>returns false - continue
    
    run->>git: getSelectedFiles(options)
    
    alt Selection Mode = Unstaged
        git->>git: getStatusDiffCommand(diffFilter, false)
        git->>git: execGitZ(git diff --name-only -z --diff-filter=... [no --staged])
    else Selection Mode = Tracked
        git->>git: getTrackedFiles()<br/>= staged + unstaged union
    else Selection Mode = All
        git->>git: getAllFiles()<br/>= tracked + untracked union
    end
    
    git-->>run: Selected files list
    run->>run: Backup stash gated on<br/>UsesIndex() [skipped for unstaged]
    run->>Task: Task: Running tasks for selected files
    Task->>Task: Execute linting on working-tree files
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 With --status flags held high,

We hop through stages, tracked and shy,

Unstaged, untracked, all the way—

New selection modes let us play!

Index flows break when needed best,

Our lint-staged now passes the test! 🌟

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title does not accurately describe the substantial changes made. The actual changeset introduces comprehensive file-selection logic for lint-staged via new --status option, selection modes, and related infrastructure changes—not just a submodule commit fix. Update the title to reflect the main change, such as 'feat: add --status option for lint-staged file selection' or 'refactor: support multiple git file selection modes in lint-staged'.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/lib/tl/list.go`:
- Around line 101-114: The loop in tl.list() discards the BreakFlow reason and
later calls task.skip(p) without a reason, so capture and propagate the
BreakFlow reason: change the BreakFlow handling inside the for loop to extract
both shouldBreak and reason (e.g., shouldBreak, skipReason := task.BreakFlow()),
store skipReason in an outer variable when shouldBreak becomes true, and change
subsequent task.skip calls to pass that reason (e.g., task.skip(p, skipReason));
also update the task.skip signature/implementations (e.g., in
internal/lib/tl/task.go) to accept and record the optional reason so the UI
shows the preserved message like "workspace is clean".
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 037bfa9c-650d-4c98-8d3a-1738beeedf8d

📥 Commits

Reviewing files that changed from the base of the PR and between f201e0c and a822de6.

📒 Files selected for processing (11)
  • README.md
  • internal/ext/lint-staged/errors.go
  • internal/ext/lint-staged/git.go
  • internal/ext/lint-staged/gitWorkflow.go
  • internal/ext/lint-staged/main.go
  • internal/ext/lint-staged/messages.go
  • internal/ext/lint-staged/run.go
  • internal/ext/lint-staged/selection.go
  • internal/ext/lint-staged/selection_test.go
  • internal/lib/tl/list.go
  • internal/lib/tl/task.go

Comment thread internal/lib/tl/list.go
Comment on lines +101 to +114
shouldBreak := false

for i, task := range tl.tasks {
if preventContinue {
if preventContinue || shouldBreak {
task.skip(p)
continue
}

taskResult := task.start(p)
result.SubResults[i] = taskResult

if task.BreakFlow != nil {
shouldBreak, _ = task.BreakFlow()
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Preserve the BreakFlow reason when skipping remaining tasks.

Line 113 discards the reason returned by BreakFlow, and Lines 104-106 skip later tasks without passing one. The lint-staged path returns "workspace is clean", but the UI will show anonymous skips.

🐛 Proposed fix
 	preventContinue := false
 	shouldBreak := false
+	breakReason := ""

 	for i, task := range tl.tasks {
 		if preventContinue || shouldBreak {
-			task.skip(p)
+			task.skip(p, breakReason)
 			continue
 		}

 		taskResult := task.start(p)
 		result.SubResults[i] = taskResult

 		if task.BreakFlow != nil {
-			shouldBreak, _ = task.BreakFlow()
+			if breakNow, reason := task.BreakFlow(); breakNow {
+				shouldBreak = true
+				breakReason = reason
+			}
 		}

Supporting change in internal/lib/tl/task.go:

-func (t *Task) skip(p *tea.Program) {
+func (t *Task) skip(p *tea.Program, reason string) {
 	p.Send(&eventTaskSkip{
-		Id: t.id,
+		Id:     t.id,
+		Reason: reason,
 	})
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/lib/tl/list.go` around lines 101 - 114, The loop in tl.list()
discards the BreakFlow reason and later calls task.skip(p) without a reason, so
capture and propagate the BreakFlow reason: change the BreakFlow handling inside
the for loop to extract both shouldBreak and reason (e.g., shouldBreak,
skipReason := task.BreakFlow()), store skipReason in an outer variable when
shouldBreak becomes true, and change subsequent task.skip calls to pass that
reason (e.g., task.skip(p, skipReason)); also update the task.skip
signature/implementations (e.g., in internal/lib/tl/task.go) to accept and
record the optional reason so the UI shows the preserved message like "workspace
is clean".

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.

2 participants