refactor(DisplaySettings): use Options for default fields #161

ada4a · 2025-01-22T11:07:50+01:00

ada4a commented

2025-01-22 11:07:50 +01:00

Previously, we needed to specify the default values in multiple places:

the two default{,_compact} constructors
in the binary, as default_*_name
also in the binary, as the default values for flags

This commit removes all those and uses Nones in there, instead adding methods called DisplaySettings::<field_name>_or_default that return Some values if they exist, or the actual defaults in case of None.

There was another approach that came to mind, but was seen as too cumbersome: make a newtype for each such field, and implement Default for them -- this'd requier too much boilerplate and could require exposing the newtypes in a lot of places (most importantly, in the binary).

Ideally, of course, we'd use default field values, but those aren't stabilzed yet.¹

This also gets rid of the TODO in mergiraf solve, which was addressed back in #74

https://github.com/rust-lang/rust/issues/132162 ↩︎

Previously, we needed to specify the default values in multiple places: - the two `default{,_compact}` constructors - in the binary, as `default_*_name` - also in the binary, as the default values for flags This commit removes all those and uses `None`s in there, instead adding methods called `DisplaySettings::<field_name>_or_default` that return `Some` values if they exist, or the _actual_ defaults in case of `None`. There was another approach that came to mind, but was seen as too cumbersome: make a newtype for each such field, and implement `Default` for them -- this'd requier too much boilerplate and could require exposing the newtypes in a lot of places (most importantly, in the binary). Ideally, of course, we'd use default field values, but those aren't stabilzed yet.[^1] This also gets rid of the TODO in `mergiraf solve`, which was addressed back in #74 [^1]: https://github.com/rust-lang/rust/issues/132162

ada4a added 1 commit

2025-01-22 11:07:51 +01:00

refactor(DisplaySettings): use Options for default fields

/ test (pull_request) Successful in 1m24s

Details

4815b1aeea

Previously, we needed to specify the default values in multiple places:
- the two `default{,_compact}` constructors
- in the binary, as `default_*_name`
- also in the binary, as the default values for flags

This commit removes all those and uses `None`s in there, instead adding
methods called `DisplaySettings::<field_name>_or_default` that return
`Some` values if they exist, or the _actual_ defaults in case of `None`.

There was another approach that came to mind, but was seen as too
cumbersome: make a newtype for each such field, and implement `Default`
for them -- this'd requier too much boilerplate and could require
exposing the newtypes in a lot of places (most importantly, in the
binary).

Ideally, of course, we'd use default field values, but those aren't
stabilzed yet.[^1]

This also gets rid of the TODO in `mergiraf solve`, which was addressed
back in #74

[^1]: https://github.com/rust-lang/rust/issues/132162

wetneb reviewed

2025-01-22 11:13:20 +01:00

src/settings.rs Outdated

					
				@ -23,1 +48,3 @@

				    pub fn left_marker(&self) -> String {

				    /// Uses the default values of `conflict_marker_size` and `left_revision_name` if not set

				    pub fn left_marker_or_default(&self) -> String {

				        println!("called left_marker_or_default");