search

Nemocas / AbstractAlgebra.jl

Generic abstract algebra functionality in pure Julia (no C dependencies)
https://nemocas.github.io/AbstractAlgebra.jl/dev/index.html
Other
163 stars 63 forks source link

Replace most remaining supercompact uses by terse #1701

Closed fingolfin closed 4 months ago

fingolfin commented 4 months ago

We retain the name :supercompact for the IOContext property for now to maximize compatibility while some of our downstream packages still use it.

Moreover, we retain the PrettyPrinting.supercompact helper for now as it is used by Nemo.

Also, while replacing a reference to supercompact in a comment to the detailed show method for maps, I've decided to fully rewrite that comment.

There is one issue with the new PrettyPrinting.terse helper which replaces PrettyPrinting.supercompact: it has the same name as the function we use to activate terse mode for IO objects... the only impact of this I can think of is that we can't use it to "terse print" IO objects -- which seems perfectly fine by me given that this function is only meant for use in test suites anyway, and we don't have reason to test the "terse printing" of IO objects (and even if we did, one could use a separate helper for that specific use case).

That said, I am open to alternative names -- e.g. terse_repr or repr_terse (then one should think about using a similar scheme for the PrettyPrinting.detailed and PrettyPrinting.oneline helpers -- or alternatively consider dropping them as one could just use repr directly)

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 50.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 87.26%. Comparing base (b2544d9) to head (c2afada).

Files Patch % Lines
src/PrettyPrinting.jl 50.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1701 +/- ## ========================================== - Coverage 87.28% 87.26% -0.02% ========================================== Files 116 116 Lines 29736 29730 -6 ========================================== - Hits 25954 25945 -9 - Misses 3782 3785 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

thofma commented 4 months ago

Curiosity. I would have implemented it just using sprint(show, ...), because repr is another one of those weird printing julia things, where it is unclear what they are supposed to do.

I don't mind too much about the name, since they are only internal.

fingolfin commented 4 months ago

Regarding repr: I wasn't really actively aware of it before today, and then just had a look at its implementation, which is

repr(x; context=nothing) = sprint(show, x; context=context)

so it seemed natural to use it.