fix: merge .gitignore instead of overwrite

This commit is contained in:
2026-08-08 13:06:48 -04:00
parent 3a7a0d5873
commit 468af3a815
+223 -2
View File
@@ -9,7 +9,9 @@
//! checking — [`validate`] for problems that must be fixed and [`lint`] for
//! item-writing guidance. [`catalog`] summarizes the pool that results.
use std::collections::BTreeMap;
use std::collections::{BTreeMap, BTreeSet};
use std::fs;
use std::path::Path;
use coursebank::assessment::AssessmentFile;
use coursebank::bank::BankFile;
@@ -44,6 +46,9 @@ reports/
*.swp
";
/// Marks the block `init` prepends to a `.gitignore` that was already there.
const GITIGNORE_HEADER: &str = "# Added by coursebank init.";
/// `init`: create a new course directory, refusing to clobber an existing one.
pub(crate) fn init(cli: &Cli, args: &InitArgs) -> Result<Outcome> {
let layout = Layout::new(&cli.course);
@@ -70,7 +75,7 @@ pub(crate) fn init(cli: &Cli, args: &InitArgs) -> Result<Outcome> {
println!("wrote {}", bank_path.display());
}
yaml::write_text(&cli.course.join(".gitignore"), GITIGNORE)?;
write_gitignore(&cli.course.join(".gitignore"))?;
println!(
"\nNext: edit {} to add your learning objectives and lectures, then\n \
coursebank bank new unit-1 --title \"Unit 1\"\n coursebank validate",
@@ -79,6 +84,152 @@ pub(crate) fn init(cli: &Cli, args: &InitArgs) -> Result<Outcome> {
Ok(Outcome::Ok)
}
/// What reconciling [`GITIGNORE`] against a file already on disk would do.
struct GitignoreMerge {
/// The file to write. Identical to the input when nothing was missing.
text: String,
/// The patterns that were missing, in the order [`GITIGNORE`] lists them.
added: Vec<String>,
/// Patterns the file un-ignores with a `!` rule, which are left out. Git
/// applies the last matching rule, so a line added at the top would lose.
negated: Vec<String>,
}
/// The comparison key for one `.gitignore` line, or `None` for a blank or comment.
///
/// `build`, `build/`, and `/build/` are one pattern spelled three ways, so the key
/// drops the slashes. A leading `!` stays, because `!build` is the opposite of
/// `build` rather than a restatement of it.
fn pattern_key(line: &str) -> Option<String> {
let trimmed = line.trim();
if trimmed.is_empty() || trimmed.starts_with('#') {
return None;
}
let (bang, rest) = match trimmed.strip_prefix('!') {
Some(rest) => ("!", rest.trim_start()),
None => ("", trimmed),
};
let rest = rest.trim_start_matches('/').trim_end_matches('/');
if rest.is_empty() {
return None;
}
Some(format!("{bang}{rest}"))
}
/// Reconciles [`GITIGNORE`] against a `.gitignore` that is already on disk.
///
/// Patterns the file already has are skipped, and a block of [`GITIGNORE`] left
/// with no patterns loses its comment too, so nobody ends up with a heading over
/// nothing. What survives goes above the existing content, which is copied
/// through unchanged, including its line endings.
fn merge_gitignore(existing: &str) -> GitignoreMerge {
let keys: BTreeSet<String> = existing.lines().filter_map(pattern_key).collect();
let crlf = existing.contains("\r\n");
let newline = if crlf { "\r\n" } else { "\n" };
let mut block: Vec<&str> = Vec::new();
let mut pending: Vec<&str> = Vec::new();
let mut added: Vec<String> = Vec::new();
let mut negated: Vec<String> = Vec::new();
let mut kept_in_block = false;
for line in GITIGNORE.lines() {
let trimmed = line.trim();
if trimmed.is_empty() {
// A blank line starts a new block, so any comment still waiting for a
// pattern belonged to a block that was dropped entirely.
pending.clear();
kept_in_block = false;
continue;
}
if trimmed.starts_with('#') {
pending.push(trimmed);
continue;
}
let Some(key) = pattern_key(trimmed) else {
continue;
};
if keys.contains(&key) {
continue;
}
if keys.contains(&format!("!{key}")) {
negated.push(trimmed.to_string());
continue;
}
if !kept_in_block && !block.is_empty() {
block.push("");
}
block.append(&mut pending);
kept_in_block = true;
block.push(trimmed);
added.push(trimmed.to_string());
}
let mut text = String::new();
if !block.is_empty() {
for line in std::iter::once(GITIGNORE_HEADER).chain(block).chain([""]) {
text.push_str(line);
text.push_str(newline);
}
}
text.push_str(existing);
GitignoreMerge {
text,
added,
negated,
}
}
/// Writes the `.gitignore`, merging into one that is already there.
///
/// `init` is often run in a repository that already has a `.gitignore`.
/// An existing file keeps everything it had and gains only the patterns
/// it was missing, at the top where they are easy to see in the diff.
fn write_gitignore(path: &Path) -> Result<()> {
let existing = match fs::read_to_string(path) {
Ok(text) => text,
// Nothing to merge with. Stay quiet about it.
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
return yaml::write_text(path, GITIGNORE);
}
// A file that is there but unreadable, or not UTF-8, is not one to
// replace on a guess.
Err(e) => return Err(Error::io(path, e)),
};
let merge = merge_gitignore(&existing);
if merge.added.is_empty() {
println!(
"{} already has every pattern init would add",
path.display()
);
} else {
yaml::write_text(path, &merge.text)?;
println!(
"added {} pattern(s) to the top of {}: {}",
merge.added.len(),
path.display(),
merge.added.join(" ")
);
}
for pattern in &merge.negated {
println!(
"note: {} un-ignores `{pattern}`, and the last matching rule wins, \
so init did not add it",
path.display()
);
if pattern.contains("salt") {
println!(
" that rule will commit the pseudonymization salt; \
remove it before you push"
);
}
}
Ok(())
}
/// `schema`: (re)write the JSON Schemas an editor uses to validate the YAML.
pub(crate) fn schema(cli: &Cli) -> Result<Outcome> {
let layout = Layout::new(&cli.course);
@@ -271,4 +422,74 @@ mod tests {
assert!(GITIGNORE.contains(".coursebank-salt"));
assert!(GITIGNORE.contains("build/"));
}
#[test]
fn merging_keeps_the_existing_file_and_adds_only_what_was_missing() {
let existing = "# rules I wrote\ntarget/\nbuild/\n*.pdf\n";
let merge = merge_gitignore(existing);
assert!(
merge.text.ends_with(existing),
"the existing file must survive byte for byte:\n{}",
merge.text
);
assert!(merge.text.starts_with(GITIGNORE_HEADER));
assert!(!merge.added.iter().any(|p| p == "build/" || p == "*.pdf"));
assert!(merge.added.iter().any(|p| p == "reports/"));
assert!(merge.added.iter().any(|p| p == ".coursebank-salt"));
}
#[test]
fn a_block_with_nothing_left_to_add_loses_its_comment() {
let merge = merge_gitignore("build/\nreports/\n");
assert!(!merge.text.contains("# Generated output"));
assert!(merge.text.contains("# Typst and PDF artifacts."));
}
#[test]
fn slashes_do_not_make_a_pattern_look_new() {
let merge = merge_gitignore("/build\nreports\n/data/\n");
assert!(
!merge.added.iter().any(|p| p.contains("build")),
"`/build` already covers `build/`, so it must not be added again"
);
assert!(!merge.added.iter().any(|p| p.contains("reports")));
}
#[test]
fn a_complete_file_is_left_exactly_as_it_was() {
let merge = merge_gitignore(GITIGNORE);
assert!(merge.added.is_empty());
assert_eq!(merge.text, GITIGNORE);
}
#[test]
fn a_negated_pattern_is_reported_instead_of_reinserted() {
let merge = merge_gitignore("*.pdf\n!*.salt\n");
assert_eq!(merge.negated, vec!["*.salt".to_string()]);
assert!(!merge.added.iter().any(|p| p == "*.salt"));
// The un-ignore covers one spelling of the salt, not the other.
assert!(merge.added.iter().any(|p| p == ".coursebank-salt"));
}
#[test]
fn line_endings_follow_the_file_being_merged_into() {
let merge = merge_gitignore("target/\r\n");
assert_eq!(
merge.text.matches('\n').count(),
merge.text.matches("\r\n").count(),
"a CRLF file must not gain bare LF lines:\n{:?}",
merge.text
);
}
#[test]
fn comments_and_blank_lines_are_not_patterns() {
assert_eq!(pattern_key(" build/ ").as_deref(), Some("build"));
assert_eq!(pattern_key("/build/").as_deref(), Some("build"));
assert_eq!(pattern_key("!build").as_deref(), Some("!build"));
assert_eq!(pattern_key("# build/"), None);
assert_eq!(pattern_key(" "), None);
assert_eq!(pattern_key("/"), None);
}
}