Proposal: Replace Text.Printf with extensible, complete version
Greetings. This proposal is an extension of previous discussion on the libraries list about replacing Text.Printf with my updated version that provides extensibility of printf to user datatypes and implements much more of the C printf spec, while maintaining backward-compatibility with existing users of printf. There is a separable proposal to move Text.Printf from base into its own package. I support that proposal, but believe that we can take them in either order, and this is the part I know how to do :-). Attached please find a patch that implements the proposal. Note that it also makes slight, backward-compatible modification to Numeric and GHC.Float to avoid a whole bunch of pointless code duplication. The printf patch is kind of ugly because so much of Text/Printf.hs has been replaced. Let me know if it would be better to just provide the whole source. The patch applies and builds against my current top-of-tree GHC. While I would welcome shortening the discussion given how much has already been discussed, Monday 30 September is two weeks from today, so I will treat that as the discussion deadline unless I hear otherwise. Thanks much for the potential opportunity to contribute to the Haskell libraries! Bart Massey bart@cs.pdx.edu
Hi Bart, Am Montag, den 16.09.2013, 18:28 -0700 schrieb Bart Massey:
Greetings. This proposal is an extension of previous discussion on the libraries list about replacing Text.Printf with my updated version that provides extensibility of printf to user datatypes and implements much more of the C printf spec, while maintaining backward-compatibility with existing users of printf.
thanks for bringing this officially to the committee’s attention. Given the backwards-compatible form of the patch, and the broad support, I believe we can go on and include this in base now. Unless someone complains today, I’ll look into pushing this tomorrow.
There is a separable proposal to move Text.Printf from base into its own package. I support that proposal, but believe that we can take them in either order, and this is the part I know how to do :-).
I don’t think we should remove it from base for this release. Hopefully we’ll tackle the larger base-splitting-and-shimming for the next release, and it would be nicer to our users to have only one round of “if you use feature x, which was removed from base, use package y”. Too bad that we don’t have module re-exports on the package level, otherwise I had suggested that you * create a package printf that re-exports Text.Printf and * base:Text.Printf gives a deprecation warning, telling people to use Text.Printf (note the same name) from the printf package. But that is a side-note and not related to the proposal at hand.
Attached please find a patch that implements the proposal. Note that it also makes slight, backward-compatible modification to Numeric and GHC.Float to avoid a whole bunch of pointless code duplication. The printf patch is kind of ugly because so much of Text/Printf.hs has been replaced. Let me know if it would be better to just provide the whole source. The patch applies and builds against my current top-of-tree GHC.
I find links to github branches (e.g. something like https://github.com/BartMassey/packages-base/compare/new-printf) nice, but of course the patch is fine. In your original mail you said that you created a testsuite. Is there a way to include them in base’s testsuite? Note that there are already Text.Printf-tests in there. But be careful about the copyright of the files you copy there; if required add their status to LICENSE. OTOH, if we expect to to separate the code again in the next release and you don’t find base’s testrunner pleasant to work with, you can skip this. Would you mind writing a sentence or short paragraph for the release notes, describing your changes?
While I would welcome shortening the discussion given how much has already been discussed, Monday 30 September is two weeks from today, so I will treat that as the discussion deadline unless I hear otherwise.
As I said, I don’t think we need the full discussion time frame, and rather try to squeeze this into base for 7.8. (My opinion, if someone disagrees please shout.) Greetings, Joachim -- Joachim Breitner e-Mail: mail@joachim-breitner.de Homepage: http://www.joachim-breitner.de ICQ#: 74513189 Jabber-ID: nomeata@joachim-breitner.de
Thanks much for the fast response!
My printf tests are at http://github.com/BartMassey/printf-tests.
They're kind of cross-implementation, so I'm not sure of the best way
to fold them into GHC: if I capture them now, GHC won't automatically
track future improvements. Give me another month to work on one more
batch of tests with my students, and I'll end up with something
GHC-ready.
I should warn you that the unit tests are most of the testing I've
done. I would be happy if someone not me compiled a few packages
against the new code and verified that there isn't some horrible
obvious thing that should be fixed before the 7.8 release. I'll try to
be especially available over the next few days for any bug reports.
If the patch is OK, I think I'll leave things that way for now. In the
future, I'll try to send a pull request instead.
Release Note: Rewrote portions of Text.Printf, and made changes to
Numeric (added Numeric.showFFloatAlt and Numeric.showGFloatAlt) and
GHC.Float (added formatRealFloatAlt) to support it. Rewritten version
is extensible to user types, adds a "generic" format specifier "%v",
extends the printf spec to support much of C printf(3) functionality,
and fixes the spurious GHC warnings about using Text.Printf.printf at
(IO a) and ignoring the return value.
--Bart
On Tue, Sep 17, 2013 at 12:30 AM, Joachim Breitner
Hi Bart,
Am Montag, den 16.09.2013, 18:28 -0700 schrieb Bart Massey:
Greetings. This proposal is an extension of previous discussion on the libraries list about replacing Text.Printf with my updated version that provides extensibility of printf to user datatypes and implements much more of the C printf spec, while maintaining backward-compatibility with existing users of printf.
thanks for bringing this officially to the committee’s attention.
Given the backwards-compatible form of the patch, and the broad support, I believe we can go on and include this in base now. Unless someone complains today, I’ll look into pushing this tomorrow.
There is a separable proposal to move Text.Printf from base into its own package. I support that proposal, but believe that we can take them in either order, and this is the part I know how to do :-).
I don’t think we should remove it from base for this release. Hopefully we’ll tackle the larger base-splitting-and-shimming for the next release, and it would be nicer to our users to have only one round of “if you use feature x, which was removed from base, use package y”.
Too bad that we don’t have module re-exports on the package level, otherwise I had suggested that you * create a package printf that re-exports Text.Printf and * base:Text.Printf gives a deprecation warning, telling people to use Text.Printf (note the same name) from the printf package.
But that is a side-note and not related to the proposal at hand.
Attached please find a patch that implements the proposal. Note that it also makes slight, backward-compatible modification to Numeric and GHC.Float to avoid a whole bunch of pointless code duplication. The printf patch is kind of ugly because so much of Text/Printf.hs has been replaced. Let me know if it would be better to just provide the whole source. The patch applies and builds against my current top-of-tree GHC.
I find links to github branches (e.g. something like https://github.com/BartMassey/packages-base/compare/new-printf) nice, but of course the patch is fine.
In your original mail you said that you created a testsuite. Is there a way to include them in base’s testsuite? Note that there are already Text.Printf-tests in there. But be careful about the copyright of the files you copy there; if required add their status to LICENSE. OTOH, if we expect to to separate the code again in the next release and you don’t find base’s testrunner pleasant to work with, you can skip this.
Would you mind writing a sentence or short paragraph for the release notes, describing your changes?
While I would welcome shortening the discussion given how much has already been discussed, Monday 30 September is two weeks from today, so I will treat that as the discussion deadline unless I hear otherwise.
As I said, I don’t think we need the full discussion time frame, and rather try to squeeze this into base for 7.8. (My opinion, if someone disagrees please shout.)
Greetings, Joachim -- Joachim Breitner e-Mail: mail@joachim-breitner.de Homepage: http://www.joachim-breitner.de ICQ#: 74513189 Jabber-ID: nomeata@joachim-breitner.de
_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries
Hi, Am Dienstag, den 17.09.2013, 01:38 -0700 schrieb Bart Massey:
If the patch is OK, I think I'll leave things that way for now. In the future, I'll try to send a pull request instead.
I get: "inplace/bin/ghc-stage1" -hisuf hi -osuf o -hcsuf hc -static -H32m -O -Werror -Wall -H64m -O0 -package-name ghc-7.7.20130917 -hide-all-packages -i -icompiler/basicTypes -icompiler/cmm -icompiler/codeGen -icompiler/coreSyn -icompiler/deSugar -icompiler/ghci -icompiler/hsSyn -icompiler/iface -icompiler/llvmGen -icompiler/main -icompiler/nativeGen -icompiler/parser -icompiler/prelude -icompiler/profiling -icompiler/rename -icompiler/simplCore -icompiler/simplStg -icompiler/specialise -icompiler/stgSyn -icompiler/stranal -icompiler/typecheck -icompiler/types -icompiler/utils -icompiler/vectorise -icompiler/stage2/build -icompiler/stage2/build/autogen -Icompiler/stage2/build -Icompiler/stage2/build/autogen -Icompiler/. -Icompiler/parser -Icompiler/utils -Icompiler/../rts/dist/build -Icompiler/stage2 -optP-DGHCI -optP-include -optPcompiler/stage2/build/autogen/cabal_macros.h -package Cabal-1.18.1 -package array-0.4.0.2 -package base-4.7.0.0 -package bin-package-db-0.0.0.0 -package bytestring-0.10.3.0 -package containers-0.5.3.1 -package directory-1.2.0.1 -package filepath-1.3.0.2 -package hoopl-3.10.0.0 -package hpc-0.6.0.1 -package process-1.2.0.0 -package template-haskell-2.9.0.0 -package time-1.4.1 -package transformers-0.3.0.0 -package unix-2.7.0.0 -Wall -fno-warn-name-shadowing -XHaskell98 -XCPP -XMagicHash -XUnboxedTuples -XPatternGuards -XForeignFunctionInterface -XEmptyDataDecls -XTypeSynonymInstances -XMultiParamTypeClasses -XFlexibleInstances -XRankNTypes -XScopedTypeVariables -XDeriveDataTypeable -XBangPatterns -XNondecreasingIndentation -optc-DTHREADED_RTS -DGHCI_TABLES_NEXT_TO_CODE -DSTAGE=2 -O2 -fwarn-tabs -fno-warn-amp -O -dcore-lint -no-user-package-db -rtsopts -odir compiler/stage2/build -hidir compiler/stage2/build -stubdir compiler/stage2/build -dynamic-too -c compiler/utils/UniqFM.lhs -o compiler/stage2/build/UniqFM.o -dyno compiler/stage2/build/UniqFM.dyn_o utils/ghc-pkg/Main.hs:1186:9: Warning: This binding for ‛toField’ shadows the existing binding imported from ‛Text.Printf’ at utils/ghc-pkg/Main.hs:27:1-18 <no location info>: Failing due to -Werror. *** Core Lint warnings : in result of Desugar (after optimization) *** {-# LINE 181 "compiler/utils/UniqFM.lhs #-}: Warning: [RHS of $c/=_a4bO :: forall ele_a2p7. GHC.Classes.Eq ele_a2p7 => UniqFM.UniqFM ele_a2p7 -> UniqFM.UniqFM ele_a2p7 -> GHC.Types.Bool] INLINE binder is (non-rule) loop breaker: $c/=_a4bO make[1]: *** [utils/ghc-pkg/dist-install/build/Main.dyn_o] Fehler 1 make[1]: *** Warte auf noch nicht beendete Prozesse... make: *** [all] Fehler 2 Maybe you need to use different names to avoid such clashes? Or provide a patch for ghc-pkg as well. In either case, a validated patch would be welcome. Greetings, Joachim -- Joachim Breitner e-Mail: mail@joachim-breitner.de Homepage: http://www.joachim-breitner.de ICQ#: 74513189 Jabber-ID: nomeata@joachim-breitner.de
Hi, Am Dienstag, den 17.09.2013, 17:19 +0200 schrieb Joachim Breitner:
Or provide a patch for ghc-pkg as well.
I’m patched ghc-pkg and check if it validates now; if it goes through I’ll push. Greetings, Joachim -- Joachim Breitner e-Mail: mail@joachim-breitner.de Homepage: http://www.joachim-breitner.de ICQ#: 74513189 Jabber-ID: nomeata@joachim-breitner.de
Sincere apologies. I had screwed up my flags in build.mk and wasn't
seeing this warning. Seeing it now.
I was afraid that "toField" is too generic a name (and not that great
a name, either)---you can change it if you prefer.
--Bart
On Tue, Sep 17, 2013 at 9:03 AM, Joachim Breitner
Hi,
Am Dienstag, den 17.09.2013, 17:19 +0200 schrieb Joachim Breitner:
Or provide a patch for ghc-pkg as well.
I’m patched ghc-pkg and check if it validates now; if it goes through I’ll push.
Greetings, Joachim
-- Joachim Breitner e-Mail: mail@joachim-breitner.de Homepage: http://www.joachim-breitner.de ICQ#: 74513189 Jabber-ID: nomeata@joachim-breitner.de
_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries
Hi, Am Dienstag, den 17.09.2013, 11:49 -0700 schrieb Bart Massey:
Sincere apologies. I had screwed up my flags in build.mk and wasn't seeing this warning. Seeing it now.
I was afraid that "toField" is too generic a name (and not that great a name, either)---you can change it if you prefer.
hmm, valid point. Maybe formatArg (as it is the function that formats the argument), which is slightly less generic. Greetings, Joachim -- Joachim Breitner e-Mail: mail@joachim-breitner.de Homepage: http://www.joachim-breitner.de ICQ#: 74513189 Jabber-ID: nomeata@joachim-breitner.de
Hi, Am Dienstag, den 17.09.2013, 21:43 +0200 schrieb Joachim Breitner:
Am Dienstag, den 17.09.2013, 11:49 -0700 schrieb Bart Massey:
Sincere apologies. I had screwed up my flags in build.mk and wasn't seeing this warning. Seeing it now.
I was afraid that "toField" is too generic a name (and not that great a name, either)---you can change it if you prefer.
hmm, valid point. Maybe formatArg (as it is the function that formats the argument), which is slightly less generic.
commited (with formatArg). On behalf of the core libraries committee: Thanks for and congrats to your first contribution to base! Greetings, Joachim -- Joachim Breitner e-Mail: mail@joachim-breitner.de Homepage: http://www.joachim-breitner.de ICQ#: 74513189 Jabber-ID: nomeata@joachim-breitner.de
Thanks much for the opportunity to contribute! I look forward to many
future bug reports. :-) --Bart
On Tue, Sep 17, 2013 at 1:18 PM, Joachim Breitner
Hi,
Am Dienstag, den 17.09.2013, 21:43 +0200 schrieb Joachim Breitner:
Am Dienstag, den 17.09.2013, 11:49 -0700 schrieb Bart Massey:
Sincere apologies. I had screwed up my flags in build.mk and wasn't seeing this warning. Seeing it now.
I was afraid that "toField" is too generic a name (and not that great a name, either)---you can change it if you prefer.
hmm, valid point. Maybe formatArg (as it is the function that formats the argument), which is slightly less generic.
commited (with formatArg).
On behalf of the core libraries committee: Thanks for and congrats to your first contribution to base!
Greetings, Joachim
-- Joachim Breitner e-Mail: mail@joachim-breitner.de Homepage: http://www.joachim-breitner.de ICQ#: 74513189 Jabber-ID: nomeata@joachim-breitner.de
_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries
participants (2)
-
Bart Massey -
Joachim Breitner