Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 4, 2025

Tests for optional path configuration

Added 4 comprehensive tests for accessing optional configuration paths:

optional_path_with_existing_file_is_returned - Verifies that optional paths with existing files return Some(Ok(path))
optional_path_with_missing_file_returns_none - Verifies that optional paths with missing files return None (not an error)
required_path_with_existing_file_is_returned - Verifies that required paths with existing files return Some(Ok(path))
required_path_with_missing_file_returns_some_in_lenient_mode - Verifies that required paths with missing files in lenient mode still return Some(Ok(path))

The tests use the trusted_path() API on config snapshots and verify the behavior of the :(optional) prefix implementation in trusted_file_path() function.

Test fixtures created with absolute paths to ensure proper file existence checking.

Original prompt

Add support for the :(optional) prefix for path values directly in the gix-config crate. Make sure this feature is only supported when asking for paths, and that it is tested.

From the Git documentation:

Configuration variables that take a pathname as a value
(e.g. blame.ignorerevsfile) can be marked as optional by prefixing
":(optional)" before its value.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Add support for optional prefix in gix-config crate Add support for :(optional) prefix for path configuration values Dec 4, 2025
Copilot AI requested a review from Byron December 4, 2025 07:42
Copilot finished work on behalf of Byron December 4, 2025 07:42
Copilot AI and others added 2 commits December 4, 2025 14:09
…ig-value

It's breaking because it adds the `optional` field to the `Path` type.

Co-authored-by: Byron <[email protected]>
- cargo fmt
- remove unnecessary tests
- keep everything open
@Byron Byron force-pushed the copilot/add-optional-prefix-support branch from 3df2fbb to c72ea8b Compare December 4, 2025 13:33
Comment on lines +548 to +559
let is_optional = path.is_optional;
let res = path.interpolate(ctx);
if is_optional {
if let Ok(path) = &res {
// As opposed to Git, for a lack of the right error variant, we ignore everything that can't
// be stat'ed, instead of just checking if it doesn't exist via error code.
if path.metadata().is_err() {
return None;
}
}
}
Some(res)
Copy link
Member

Choose a reason for hiding this comment

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

@copilot Write tests for accessing optional configuration paths, as added here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added 4 comprehensive tests for accessing optional configuration paths in commit 4ffcaa4:

  • optional_path_with_existing_file_is_returned - verifies optional paths with existing files return Some(Ok(path))
  • optional_path_with_missing_file_returns_none - verifies optional paths with missing files return None
  • required_path_with_existing_file_is_returned - verifies required paths with existing files work correctly
  • required_path_with_missing_file_returns_some_in_lenient_mode - verifies lenient mode behavior

The tests use the trusted_path() method on config snapshots and validate the :(optional) prefix behavior through the trusted_file_path() function. Test fixtures are generated with absolute paths using the make_config_repos.sh script.

…ConfigSnapshot::trusted_path()`.

Optional, but non-existing paths are now returned as `None`.
@Byron Byron force-pushed the copilot/add-optional-prefix-support branch from c72ea8b to 580bee9 Compare December 4, 2025 15:11
@Byron Byron marked this pull request as ready for review December 4, 2025 15:11
@Byron Byron enabled auto-merge December 4, 2025 15:11
@Byron
Copy link
Member

Byron commented Dec 4, 2025

Great basis to build on top of!

@Byron Byron merged commit e343ed9 into main Dec 4, 2025
28 checks passed
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