Skip to content

Add @battery / @enumbattery: opt-in counterparts to @batteries#19

Merged
jw3126 merged 6 commits into
jw3126:masterfrom
PatrickHaecker:battery
May 14, 2026
Merged

Add @battery / @enumbattery: opt-in counterparts to @batteries#19
jw3126 merged 6 commits into
jw3126:masterfrom
PatrickHaecker:battery

Conversation

@PatrickHaecker

@PatrickHaecker PatrickHaecker commented May 7, 2026

Copy link
Copy Markdown

@batteries derives a curated set of methods by default; @battery does
the opposite — every default flips to false, so only the explicitly
listed batteries are derived. The bare-flag shorthand from #18
makes call sites read as a list:

    @battery T kwconstructor          # only the kwarg constructor
    @battery T eq isequal hash        # only structural ==/isequal/hash
    @battery T hash typesalt=0xab     # only stable hash

This avoids side-effects with subset use cases of @batteries
(@batteries T showrepr=true silently also defines ==, hash,
isequal, getproperties, constructorof, selfconstructor):
users who want exactly one or two batteries can now ask for just
those, with no surprise additions now or additional surprises
in the future when the defaults change.

@enumbattery is the analogous opt-in form of @enumbatteries.

Implementation factors the macro bodies into shared codegen helpers
that take the base named tuple. The 'all-false' bases are derived
programmatically from the existing DEFAULTS so they stay in sync if a
new option is added later.

@PatrickHaecker

PatrickHaecker commented May 7, 2026

Copy link
Copy Markdown
Author

I rebased this on #20 to get a green CI. So

before tackling #19.

@jw3126

jw3126 commented May 7, 2026

Copy link
Copy Markdown
Owner

Awesome the opt in variant is a great idea. We definitely need that! #18 (comment) is a good argument in favor of the new syntax.

I want to explore a few more ideas before deciding on the syntax question. For me very often I have
a whole bunch of structs for which I would like to have the same batteries options. Typically, I want the same options for almost all structs in a package, except for a few outliers.

I was wondering if we should support something like:

const config = (kwshow=true, kwconstructor=true, eq=false)

struct S1 end
@batteries S config

struct S2 end
@batteries S config typesalt=0xdeadbeef

struct S3 end
@battery S3 config kwshow=true

Or we could make it really easy for the user to customize their own macro:

macro mybatteries(args...)
    defaults = (;StructHelpers.BATTERIES_DEFAULTS..., kwshow=true, kwconstructor=true, eq=false)
    # defaults = (;StructHelpers.BATTERIES_NONE..., kwshow=true, kwconstructor=true)
    return StructHelpers.batteriesmacro(args...; defaults)
end

And then everything is decorated with @mybatteries. Thoughts?

PatrickHaecker pushed a commit to PatrickHaecker/StructHelpers.jl that referenced this pull request May 8, 2026
@PatrickHaecker

Copy link
Copy Markdown
Author

Typically, I want the same options for almost all structs in a package, except for a few outliers.

That's a fantastic idea! NamedTuples are an elegant way to configure this. I merged our proposals which I think gets us the best of both worlds.

I am not sure that we need to do anything about custom macros. The user can already define their own macro which calls into @battery or @batteries handing over some extra option.

@jw3126

jw3126 commented May 8, 2026

Copy link
Copy Markdown
Owner

This is very nice, thanks a lot for the contribution!

@jw3126

jw3126 commented May 8, 2026

Copy link
Copy Markdown
Owner

Can you bump the version?

Comment thread src/StructHelpers.jl
@PatrickHaecker

Copy link
Copy Markdown
Author

This is very nice, thanks a lot for the contribution!
Can you bump the version?

Thanks, done

PatrickHaecker pushed a commit to PatrickHaecker/StructHelpers.jl that referenced this pull request May 8, 2026
@jw3126

jw3126 commented May 10, 2026

Copy link
Copy Markdown
Owner

I am not yet sold that we should have the eq, hash magic. I would be happy to merge the rest though. Could you let claude split the PR?
You might convince me later that we want eq, hash magic by showing me some real world code where spelling the =true out causes big pain.

@PatrickHaecker

PatrickHaecker commented May 12, 2026

Copy link
Copy Markdown
Author

I am not yet sold that we should have the eq, hash magic. I would be happy to merge the rest though. Could you let claude split the PR?

I am not exactly sure whether I understand this correctly.

Do you mean #18 with "eq, hash magic"? Note that this is effectively a single line of code (line 281 in #18) which is needed for the config feature in #19 anyway. So I would need to add more code to special case this. Or would you propose that you would also write @battery MyType config=true as a derivation from your example in #19? This would read quite artificial to me.

Note, that there is quite some precedence for this style, e.g. with

Base.@assume_effects :nothrow :terminates_globally f(x) = ...

(not using :nothrow=true) or

using Foo: a, b, c

where you just list the bindings you want to have in your namespace similarly how we do. Or take Rust's derive where you just list the traits you want derived, which is basically what @battery does:

#[derive(Debug, Clone, Hash, PartialEq, Eq)]

also, no =true anywhere.

You might convince me later that we want eq, hash magic by showing me some real world code where spelling the =true out causes big pain.

"Big pain" is relative and might be a high bar:
Unfortunately, I can't share this code, but we have a repository here, where a quick search brought up 26 lines where we'd need to add a =true. This would only add visual noise without bringing any benefit. The alternative (if configs stay without =true) would be to add a config alias for a single option, which also does not seem to be helping code readability.
Compare this with the single line of code in StructHelpers.jl (not counting the comments, docstring and tests), the trade-off looks simple to me from a software quality perspective, but I might be missing something.

All this was assuming what you might have meant and I might be off. Do you think it might help to have a call? It would probably help me understanding your perspective. If you think this is helpful, I can send you a direct message via Julia's Discourse.

@jw3126

jw3126 commented May 12, 2026

Copy link
Copy Markdown
Owner

Yes this is what I meant. My biggest concern is complexity creep for the reader. Now the reader needs to know about a new syntax special case.

@PatrickHaecker

Copy link
Copy Markdown
Author

Ah, I guess this is a matter of perspective:

  • With the config the user needs to know that it is supported (mandated?) to not write =true when using the config, but not when not using the config, so I think what you propose is more complex for the user
  • We need to special-case the config case, so I think what you propose is more complex to maintain
  • The pattern to just name instead of name=true is so common, that a lot of users (most?) will expect it and are surprised that it won't work (I am glad I don't always need to type ls -l=true on the command line).
  • The code looks to me perfectly readable, so I would be surprised if a user would need to look up what @battery Params showrepr means when they know what @battery Params showrepr=true means.
  • It does not add complexity to code writers because they can use whatever they prefer.

But anyway, you are the maintainer and we can use my branch where we have the feature. So I would rebase the PRs so that they are #19 -> #21 -> #18, correct? Then you can close #18 as "not planned" and we (and whoever else wants to avoid the redundancy) can use the branch from #18.

@jw3126

jw3126 commented May 12, 2026

Copy link
Copy Markdown
Owner

I am always thinking of C++. Everybody got their favorite feature in and now there is tremendous complexity and foot guns.
While I am not a huge fan, I am okay with this feature. As you argued it is reasonable intuitive and not much complex in implementation. Could you let Claude resolve the merge conflicts and we go with this?

Patrick Häcker and others added 4 commits May 13, 2026 07:40
A bare symbol is now accepted as shorthand for \`flag=true\`:

    @Batteries T kwconstructor   # equivalent to kwconstructor=true
    @Batteries T kwconstructor kwshow hash=false   # mixable

Also unblocks the cleaner subset-mode call shape we want for an
upcoming \`@battery\` macro that derives a single hand-picked battery.
@Batteries derives a curated set of methods by default; @battery does
the opposite — every default flips to false, so only the explicitly
listed batteries are derived. The bare-flag shorthand from the previous
PR makes call sites read as a checklist:

    @battery T kwconstructor          # only the kwarg constructor
    @battery T eq isequal hash        # only structural ==/isequal/hash
    @battery T hash typesalt=0xab     # only stable hash

This addresses the side-effect concern with subset use cases of
@Batteries (\`@batteries T showrepr=true\` silently also defines
==, hash, isequal, getproperties, constructorof, selfconstructor):
users who want exactly one or two batteries can now ask for just
those, with no surprise additions now or additional surprises
in the future when the defaults change.

@enumbattery is the analogous opt-in form of @enumbatteries.

Implementation factors the macro bodies into shared codegen helpers
that take the base named tuple. The 'all-false' bases are derived
programmatically from the existing DEFAULTS so they stay in sync if a
new option is added later.

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
PatrickHaecker pushed a commit to PatrickHaecker/StructHelpers.jl that referenced this pull request May 14, 2026
Closes jw3126#12.

This is the real PR I wanted to do. All the others are basically there
to enable this one.

Introduces a new `@batteries` / `@battery` flag `showrepr=true` that
overloads `Base.show` to print a heuristically short constructor call
which round-trips through `eval`, as an alternative to the existing
`kwshow`. The two are mutually exclusive (both target `Base.show`).

I originally wanted to just extend `kwshow`. However, as the
redundancy reduction should also be available if there is no keyword
constructor and/or calls are short when using a parametric
constructor, this would have lead to broken backwards-compatibility
requiring a major version bump and user changes. But as both
approaches have different advantages, it's probably a good thing
to have both in the future.

Highlights:

* Probes every constructor of the type — positional, keyword, and
  hybrid (including the pair synthesized by `Base.@kwdef`) — and picks
  the shortest call whose result is `repr_eq` to the original. Trailing
  fields whose values match a default are omitted.
* Per-field literal shortening where the constructor still accepts the
  substitute: `0x000000000000002a` → `42`, whole-valued floats drop the
  `.0` suffix, `2//1` → `2`, `2 + 0im` → `2`.
* Uniform / run-length-compressed vectors render as `fill(v, n)` (or
  `[v for _=1:n]` for non-`isbits` elements, to avoid aliasing on
  re-eval); heterogeneous vectors get per-run RLE only when it actually
  shortens the literal.
* Round-trip safety is the gating criterion throughout: every
  substitution is verified by reconstructing through the constructor
  and comparing via `repr_eq` (which accepts both `==` and `isequal` to
  handle `NaN`, `-0.0`, and custom equality).
* Falls back to a non-executable `T(field = value, …)` rendering when
  no constructor recreates the object.

`kwshow` keeps its existing fixed `T(f1 = v1, …)` shape and stability
guarantees; `showrepr`'s output is heuristic and not pinned across
minor releases. The README contrasts the two.

This builds on jw3126#18 and jw3126#19 which should be merged first.

Co-authored-by: Copilot <copilot@github.com>
@PatrickHaecker

Copy link
Copy Markdown
Author

I think all conflicts are removed and we have everything on the branch where it belongs. Just give me a hint whether I should do further changes (comments too long? too many? testset names too long? …).

Thanks a lot for giving this feature a place upstream. I really appreciate this. I totally agree that we want to avoid C++, but I think with this feature we are still a bit away from the dark side. :-)

@jw3126 jw3126 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks a lot for implementing this. It is a nice improvement!

@jw3126
jw3126 merged commit 457a737 into jw3126:master May 14, 2026
4 checks passed
PatrickHaecker pushed a commit to PatrickHaecker/StructHelpers.jl that referenced this pull request May 15, 2026
Closes jw3126#12.

This is the real PR I wanted to do. All the others are basically there
to enable this one.

Introduces a new `@batteries` / `@battery` flag `showrepr=true` that
overloads `Base.show` to print a heuristically short constructor call
which round-trips through `eval`, as an alternative to the existing
`kwshow`. The two are mutually exclusive (both target `Base.show`).

I originally wanted to just extend `kwshow`. However, as the
redundancy reduction should also be available if there is no keyword
constructor and/or calls are short when using a parametric
constructor, this would have lead to broken backwards-compatibility
requiring a major version bump and user changes. But as both
approaches have different advantages, it's probably a good thing
to have both in the future.

Highlights:

* Probes every constructor of the type — positional, keyword, and
  hybrid (including the pair synthesized by `Base.@kwdef`) — and picks
  the shortest call whose result is `repr_eq` to the original. Trailing
  fields whose values match a default are omitted.
* Per-field literal shortening where the constructor still accepts the
  substitute: `0x000000000000002a` → `42`, whole-valued floats drop the
  `.0` suffix, `2//1` → `2`, `2 + 0im` → `2`.
* Uniform / run-length-compressed vectors render as `fill(v, n)` (or
  `[v for _=1:n]` for non-`isbits` elements, to avoid aliasing on
  re-eval); heterogeneous vectors get per-run RLE only when it actually
  shortens the literal.
* Round-trip safety is the gating criterion throughout: every
  substitution is verified by reconstructing through the constructor
  and comparing via `repr_eq` (which accepts both `==` and `isequal` to
  handle `NaN`, `-0.0`, and custom equality).
* Falls back to a non-executable `T(field = value, …)` rendering when
  no constructor recreates the object.

`kwshow` keeps its existing fixed `T(f1 = v1, …)` shape and stability
guarantees; `showrepr`'s output is heuristic and not pinned across
minor releases. The README contrasts the two.

This builds on jw3126#18 and jw3126#19 which should be merged first.

Co-authored-by: Copilot <copilot@github.com>
PatrickHaecker pushed a commit to PatrickHaecker/StructHelpers.jl that referenced this pull request May 17, 2026
Closes jw3126#12.

This is the real PR I wanted to do. All the others are basically there
to enable this one.

Introduces a new `@batteries` / `@battery` flag `showrepr=true` that
overloads `Base.show` to print a heuristically short constructor call
which round-trips through `eval`, as an alternative to the existing
`kwshow`. The two are mutually exclusive (both target `Base.show`).

I originally wanted to just extend `kwshow`. However, as the
redundancy reduction should also be available if there is no keyword
constructor and/or calls are short when using a parametric
constructor, this would have lead to broken backwards-compatibility
requiring a major version bump and user changes. But as both
approaches have different advantages, it's probably a good thing
to have both in the future.

Highlights:

* Probes every constructor of the type — positional, keyword, and
  hybrid (including the pair synthesized by `Base.@kwdef`) — and picks
  the shortest call whose result is `repr_eq` to the original. Trailing
  fields whose values match a default are omitted.
* Per-field literal shortening where the constructor still accepts the
  substitute: `0x000000000000002a` → `42`, whole-valued floats drop the
  `.0` suffix, `2//1` → `2`, `2 + 0im` → `2`.
* Uniform / run-length-compressed vectors render as `fill(v, n)` (or
  `[v for _=1:n]` for non-`isbits` elements, to avoid aliasing on
  re-eval); heterogeneous vectors get per-run RLE only when it actually
  shortens the literal.
* Round-trip safety is the gating criterion throughout: every
  substitution is verified by reconstructing through the constructor
  and comparing via `repr_eq` (which accepts both `==` and `isequal` to
  handle `NaN`, `-0.0`, and custom equality).
* Falls back to a non-executable `T(field = value, …)` rendering when
  no constructor recreates the object.

`kwshow` keeps its existing fixed `T(f1 = v1, …)` shape and stability
guarantees; `showrepr`'s output is heuristic and not pinned across
minor releases. The README contrasts the two.

This builds on jw3126#18 and jw3126#19 which should be merged first.

Co-authored-by: Copilot <copilot@github.com>
PatrickHaecker pushed a commit to PatrickHaecker/StructHelpers.jl that referenced this pull request May 18, 2026
Closes jw3126#12.

This is the real PR I wanted to do. All the others are basically there
to enable this one.

Introduces a new `@batteries` / `@battery` flag `showrepr=true` that
overloads `Base.show` to print a heuristically short constructor call
which round-trips through `eval`, as an alternative to the existing
`kwshow`. The two are mutually exclusive (both target `Base.show`).

I originally wanted to just extend `kwshow`. However, as the
redundancy reduction should also be available if there is no keyword
constructor and/or calls are short when using a parametric
constructor, this would have lead to broken backwards-compatibility
requiring a major version bump and user changes. But as both
approaches have different advantages, it's probably a good thing
to have both in the future.

Highlights:

* Probes every constructor of the type — positional, keyword, and
  hybrid (including the pair synthesized by `Base.@kwdef`) — and picks
  the shortest call whose result is `repr_eq` to the original. Trailing
  fields whose values match a default are omitted.
* Per-field literal shortening where the constructor still accepts the
  substitute: `0x000000000000002a` → `42`, whole-valued floats drop the
  `.0` suffix, `2//1` → `2`, `2 + 0im` → `2`.
* Uniform / run-length-compressed vectors render as `fill(v, n)` (or
  `[v for _=1:n]` for non-`isbits` elements, to avoid aliasing on
  re-eval); heterogeneous vectors get per-run RLE only when it actually
  shortens the literal.
* Round-trip safety is the gating criterion throughout: every
  substitution is verified by reconstructing through the constructor
  and comparing via `repr_eq` (which accepts both `==` and `isequal` to
  handle `NaN`, `-0.0`, and custom equality).
* Falls back to a non-executable `T(field = value, …)` rendering when
  no constructor recreates the object.

`kwshow` keeps its existing fixed `T(f1 = v1, …)` shape and stability
guarantees; `showrepr`'s output is heuristic and not pinned across
minor releases. The README contrasts the two.

This builds on jw3126#18 and jw3126#19 which should be merged first.

Co-authored-by: Copilot <copilot@github.com>
@PatrickHaecker
PatrickHaecker deleted the battery branch May 19, 2026 06:39
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