Skip to content

Stop a blank line in .ignore from excluding the whole Edge App - #321

Open
rusko124 wants to merge 2 commits into
masterfrom
fix/ignore-blank-lines
Open

Stop a blank line in .ignore from excluding the whole Edge App#321
rusko124 wants to merge 2 commits into
masterfrom
fix/ignore-blank-lines

Conversation

@rusko124

@rusko124 rusko124 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

A blank line in .ignore became the regex pattern ^$. collect_paths_for_upload walks with WalkDir::new(root).filter_entry(is_included), and walkdir yields the root directory itself as the first entry — Ignorer::is_ignored strips the base path off it, leaving the empty string, which ^$ matches. Rejecting a directory in filter_entry prunes its whole subtree, so the walk ended before it started and the deploy collected zero files. The server then rejects the deploy with index.html is required.

Any editor that leaves a trailing newline on the file produces this, so it is easy to hit and gives no hint where the problem is. A single trailing \n is harmless — str::lines yields no empty item for it — but \n\n is enough.

Two changes:

  • Blank (and whitespace-only) lines are skipped when patterns are built. An empty pattern has no meaning in any ignore syntax.
  • is_included exempts the walk root, so no future pattern that happens to match the empty string can prune the whole tree again.

An empty line became the pattern ^$, and walkdir yields the root
directory itself with an empty relative path, so filter_entry pruned the
entire tree before it was walked. A deploy then collected zero files and
the server rejected it with "index.html is required". Any editor that
leaves a trailing newline produced this.

Blank lines are now skipped, and the walk root is exempt from the ignore
rules so no future pattern can prune everything the same way.
Copilot AI lite review requested due to automatic review settings September 7, 2026 11:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Directory ignore patterns ending with / still won’t match/prune the directory entry itself in filter_entry, causing potentially expensive descent into ignored trees (e.g., node_modules).

Pull request overview

Fixes a deploy-breaking edge case where blank/whitespace-only lines in .ignore could produce an empty regex (^$) that matched the walk root and caused WalkDir::filter_entry to prune the entire directory tree, resulting in zero collected upload files.

Changes:

  • Skip blank (and whitespace-only) .ignore lines when building ignore patterns to avoid generating an empty-match regex.
  • Ensure the walk root (depth 0) is always included so the traversal cannot be pruned at the root by any ignore match.
  • Add/adjust tests to cover .ignore files containing blank lines during both ignore parsing and file collection.
File summaries
File Description
src/commands/ignorer.rs Skips empty/whitespace-only .ignore lines and adds a regression test for blank-line behavior.
src/commands/edge_app/utils.rs Prevents WalkDir root pruning by always including depth-0 entries; updates ignore-related test data to include blank lines.
Review details

Suppressed comments (1)

src/commands/ignorer.rs:29

  • Directory patterns (lines ending with /) compile to a regex that requires a trailing slash (e.g., ^node_modules/.*$), but WalkDir directory entries are represented without a trailing slash (e.g., node_modules). This means ignored directories won’t be pruned by filter_entry, and the walk will still descend into large trees like node_modules, skipping files one-by-one (potentially very slow). Consider matching both the directory entry itself and everything under it.
                if pattern.ends_with('/') {
                    patterns.push(format!("^{}.*$", regex::escape(pattern)));
                } else if pattern.contains('*') {
                    // Convert wildcard '*' to regex '.*'
                    let converted = pattern.replace('.', r"\.").replace('*', r".*");
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI review requested due to automatic review settings September 7, 2026 11:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The changes directly address the identified failure mode with targeted, low-risk logic and accompanying regression tests.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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