On Thu Mar 6, 2025 at 12:57 AM CET, Danilo Krummrich wrote:
> On Wed, Mar 05, 2025 at 11:36:54PM +0000, Benno Lossin wrote:
>> On Wed Mar 5, 2025 at 11:38 PM CET, Danilo Krummrich wrote:
>> > On Wed, Mar 05, 2025 at 10:30:31PM +0000, Benno Lossin wrote:
>> >> On Tue Mar 4, 2025 at 6:34 PM CET, Danilo Krummrich wrote:
>> >> > + /// Push an additional path component.
>> >> > + ///
>> >> > + /// After a new [`ModInfoBuilder`] instance has been created, [`ModInfoBuilder::prepare`] must
>> >> > + /// be called before adding path components.
>> >> > + pub const fn push(self, s: &str) -> Self {
>> >> > + if N != 0 && self.n == 0 {
>> >> > + crate::build_error!("Must call prepare() before push().");
>> >>
>> >> This will only prevent the first `prepare` call being missed, right?
>> >
>> > Correct, unfortunately there's no way to detect subsequent ones.
>>
>> Does it make sense to do that one in the constructor?
>>
>> (After looking at the example below) Ah maybe you can't do that, since
>> then you would have two `prepare()` calls for the example below...?
>
> Exactly.
>
>> >> If you always have to call this before `push`, why not inline it there?
>> >
>> > You can push() multiple times to compose the firmware path string (which is the
>> > whole purpose :).
>>
>> Ah I see, I only looked at the example you have in the next patch. All
>> in all, I think this patch could use some better documentation, since I
>> had to read a lot of the code to understand what everything is supposed
>> to do...
>
> I can expand the example in module_firmware! to make things a bit more obvious.
>
> Otherwise, what information do you think is missing?
Abstractly: what `ModInfoBuilder` *does*, concretely:
- why the generic constant `N` exists,
- what `prepare()` does,
- what happens with the `module_name` parameter of `new`
- answer the question "I want that the builder outputs the string `???`
can it do that? If yes, how do I do it?"
>> It might also make sense to make this more generic, since one probably
>> also needs this in other places? Or do you think this will only be
>> required for modinfo?
>
> Currently, I don't think there's any more need for a generic const string
> builder. For now, I think we're good. Let's factor it out, once we have actual
> need for that.
Sounds good.
---
Cheers,
Benno
