[darcs-users] roadmap / priorities for Darcs (2016 edition!)

[Francesco Ariis]

On Thu, Sep 08, 2016 at 02:29:24PM -0300, Guillaume Hoffmann wrote:
> * since we now require GHC >= 7.10 we can grep through the TODO's in
> the source code and try to fix those that wish for new GHC features

<shameless-plug>
Why grep when you can have a better tool [1]! ;)
</shameless-plug>

I ran lentil through darcs codebase and came up with a whopping
395-lines file, which I attach.

[1] http://hackage.haskell.org/package/lentil
-------------- next part --------------
Setup.lhs
   347  take verbosity from the args.
 
darcs.cabal
   379  try to abstract this out better
 
harness/Darcs/Test/Patch.hs
   147  <no description> [xxx]
 
harness/Darcs/Test/Patch/Arbitrary/Generic.hs
    30  this is more or less a hack import Darcs.ColorPrinter ( errorDoc )
        import Darcs.ColorPrinter ( traceDoc ) [xxx]
 
harness/Darcs/Test/Patch/Arbitrary/PrimFileUUID.hs
     1  Remove these warning disabling flags...
   141  demanifest
 
harness/Darcs/Test/Patch/Arbitrary/RepoPatchV1.hs
    60  there is a lot of overlap in testing between between this module
        and Darcs.Test.Patch.QuickCheck
 
harness/Darcs/Test/Patch/Check.hs
    64  the way that the standard way to use PatchCheck is by returning
        PatchCheck Bool but then often ignoring the result and instead
        checking again for state consistency is weird. It should be
        possible to replace it by a more normal error handling mechanism.
 
harness/Darcs/Test/Patch/Properties/V1Set2.hs
    33  these are exported temporarily to mark them as used Figure out
        whether to enable or remove the tests.
 
harness/test.hs
   128  add a 'all' option (implement using an Enum instance)?
 
src/Darcs/Patch/Bracketed/Instances.hs
    18  see if this can be simplified
 
src/Darcs/Patch/Bundle.hs
   162  use EmptyCase with GHC 7.8+
 
src/Darcs/Patch/Choices.hs
   309  stop after having seen the patch we want to force first
 
src/Darcs/Patch/Info.hs
   118  <no description>
 
src/Darcs/Patch/Match.hs
   269  would this be better defined in Darcs.Commands.Help? | The string
        that is emitted when the user runs @darcs help --match at . [fixme]
   299  it would be nice to have a variable name here: "author REGEX -
        match against author (email address)" or "exact STRING - match
        against exact patch name". [fixme]
 
src/Darcs/Patch/Named/Wrapped.hs
    71  this should always be the "internal implementation detail" rebase
        patch description, so could be replaced by just the Ignore-this and
        Date fields
    85  use Data.Type.Equality and PolyKinds from GHC 7.8/base 4.7
   203  is this sensible?
   255  switch to a more natural on-disk structure that directly
        saves/reads 'RebaseP'.
 
src/Darcs/Patch/OldDate.hs
   148  In case you ever want to use this outside of darcs, you should note
        that this implementation of ISO 8601 is not complete. [fixme]
 
src/Darcs/Patch/Prim/FileUUID/Apply.hs
    46  <no description>
 
src/Darcs/Patch/Prim/FileUUID/Coalesce.hs
    11  <no description>
 
src/Darcs/Patch/Prim/FileUUID/Commute.hs
    24  we eventually have to get rid of runCommute with this signature,
        since m might involve IO at some point, which we can't "run";
        alternatively, for IO it could always yield Nothing, having a
        separate IO-specific function to "run" commutes in IO
    39  <no description>
 
src/Darcs/Patch/Prim/FileUUID/Core.hs
    43  elaborate
    72  String is not the right type here. However, what it represents is a
        single file *name* (not a path). No slashes allowed, no "." and
        ".." allowed either.
    96  PrimClassify doesn't make sense for FileUUID prims
   109  PrimConstruct makes no sense for FileUUID prims
   139  (used for --match 'hunk ...', presumably)
 
src/Darcs/Patch/Prim/FileUUID/Details.hs
    10  <no description>
 
src/Darcs/Patch/Prim/FileUUID/Read.hs
    48  a bytestring version of decodeWhite from Darcs.FileName [xxx]
 
src/Darcs/Patch/Prim/FileUUID/Show.hs
    26  this instance shouldn't really be necessary, as Prims aren't used
        generically
    64  a bytestring version of encodeWhite from Darcs.FileName [xxx]
 
src/Darcs/Patch/Prim/V1/Show.hs
    38  this instance shouldn't really be necessary, as Prims aren't used
        generically
 
src/Darcs/Patch/Rebase/Container.hs
    45  move some of the docs of types to individual constructors once
        http://trac.haskell.org/haddock/ticket/43 is fixed.
   134  ideally we would apply ps in a sandbox to check the individual
        patches are consistent with each other.
 
src/Darcs/Patch/Rebase/Item.hs
   160  sort out summaries properly, considering expected conflicts
 
src/Darcs/Patch/Rebase/Name.hs
    71  improve this?
 
src/Darcs/Patch/Rebase/Viewing.hs
   112  merge with RebaseSelect. |Used for displaying during 'rebase
        changes'. 'Named (RebaseChange p)' is very similar to 'RebaseSelect
        p' but slight mismatches ('Named' embeds an 'FL') makes it not
        completely trivial to merge them.
   439  consistency checking?
   440  consider inverse commutes, e.g. what happens if we wanted to
        commute (WithDroppedDeps ... [n]
 
src/Darcs/Patch/V1/Commute.hs
   122  when can the first attempt fail, but the second not? What's so
        clever in this function?
   312  why does this code throw away the other branch, only for merge to
        rebuild it?
 
src/Darcs/Patch/V2/Non.hs
   182  Figure out what remNons is for; it's is only used in one place -
        when commuting two Conflictors:
   224  understand if there is any case where p is equal to the prim patch
        of the Non, in which case, we return the original Non, is that
        right?
 
src/Darcs/Patch/V2/RepoPatch.hs
   683  what is foo? Why do we need nyy? I think @x'@ is @x@ in the context
        of @yy at .
   926  !!! ??? [fixme]
 
src/Darcs/Repository.hs
   215  add witnesses for pending so we can make the types precise:
        currently the passed patch can be applied in any context, not just
        after pending.
 
src/Darcs/Repository/Cache.hs
   285  document @oos@: what happens when we only speculate?
   422  create links in caches [fixme]
   504  create links in caches [fixme]
 
src/Darcs/Repository/Clone.hs
   251  : - ( [fixme]
 
src/Darcs/Repository/Internal.hs
   418  see also Repository.State.readPendingLL ... to be removed after GHC
        7.2
   688  re-add a safety catch for --dry-run? Maybe using a global, like
        dryRun :: Bool, with dryRun = unsafePerformIO $ readIORef ...
   810  this is a rather weird API. If called with a patch that isn't
        already in the repo, it fails with an obscure error from
        'commuteToEnd'. It also ends up redoing the work that the caller
        has already done - if it has already commuted these patches to the
        end, it must also know the commuted versions of the other patches
        in the repo. |Given a sequence of patches anchored at the end of
        the current repository, actually pull them to the end of the
        repository by removing any patches with the same name and then
        adding the passed in sequence. Typically callers will have obtained
        the passed in sequence using 'findCommon' and friends. [fixme]
   877  rename this and document the transaction protocol (revert/finalize)
        clearly. |Slightly confusingly named: as well as throwing away any
        tentative changes, revertRepositoryChanges also re-initialises the
        tentative state. It's therefore used before makign any changes to
        the repo.
 
src/Darcs/Repository/Job.hs
   113  Unbind Tree from RepoJob, possibly renaming existing RepoJob
 
src/Darcs/Repository/Merge.hs
   119  it's not really clear why if this is an optimisation in practice,
        as pc would be trivial to calculate in this case and there aren't
        any obvious memory benefits.
   127  we end up applying the merged remote patches on top of the unmerged
        local patches, then commuting out the unmerged local patches and
        finally adding the merged local patches. It would better to just
        remove the unmerged local patche, then apply the unmerged remote
        patches and then the merged local patches. The handling of
        'unrecorded' might complicate this slightly so this refactoring may
        be better deferred until we have reliable witness tracking for
        repositories.
 
src/Darcs/Repository/Rebase.hs
   213  this isn't under the repo lock, and it should be
 
src/Darcs/Repository/Repair.hs
   154  is the latter condition needed? Does a broken patch imply pristine
        difference? Why, or why not?
 
src/Darcs/Repository/Resolution.hs
    90  remove the following two once we can rely on GHC 7.2 / superclass
        equality
 
src/Darcs/Repository/State.hs
   104  We wrap the pending patch inside RepoPatch here, to avoid the
        requirement to propagate an (ApplyState (PrimOf p) ~ ApplyState p)
        constraint everywhere. When we have GHC 7.2 as a minimum
        requirement, we can lift this constraint into RepoPatch superclass
        context and remove this hack.
   244  untangle the arguments and make this more orthogonal
 
src/Darcs/UI/ApplyPatches.hs
   143  I suggest people writing such code should *at least* put in some
        comments. It is unclear how this works and how the intertwined
        exception handlers make this do what the author wanted.
 
src/Darcs/UI/Commands/Add.hs
   185  do not expand here, and use findM/findIO or such later (needs
        adding to hashed-storage first though)
 
src/Darcs/UI/Commands/Annotate.hs
   149  need to decide about the --machine flag
 
src/Darcs/UI/Commands/Apply.hs
   295  quadratic? [fixme]
 
src/Darcs/UI/Commands/Clone.hs
   341  remove this when we get rid of directly looking at [DarcsFlag]
 
src/Darcs/UI/Commands/Convert.hs
   431  this unsafeCoerce hack is because we don't keep track of the
        repository state properly Really sequence_ $ mapFL applySome below
        should instead be a repeated add operation - there doesn't seem to
        be any reason we need to do a merge here.
   565  is this valid? [fixme]
   573  forbidden characters and subsequences in tags:
        https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html
        [fixme]
   729  implement --dry-run, which would be read-only?
  1009  we ignore branch for now [fixme]
 
src/Darcs/UI/Commands/Help.hs
   183  the "grouping" comments below should made subsections in the
        manpage, as we already do for DarcsCommand groups. --twb, 2009
        [fixme]
   231  this is copy-and-pasted from darcs.cabal, so it'll get out of date
        as people forget to maintain both in sync. [fixme]
   261  this is a kludge. [fixme]
   276  new project: init, rec -la; track upstream project: clone, pull -a;
        contribute to project: add, rec, push/send. [fixme]
 
src/Darcs/UI/Commands/Rebase.hs
   469  should catch logfiles (fst value from updatePatchHeader) and clean
        them up as in AmendRecord
   503  should catch logfiles (fst value from updatePatchHeader) and clean
        them up as in AmendRecord
   577  this selection doesn't need to respect dependencies
   578  we only want to select one patch: generalise
        withSelectedPatchFromRepo
   659  this selection doesn't need to respect dependencies
   952  we assume the options apply only to the main command, review if
        there are any we should keep
   966  This is a nasty hack, caused by the fact that readUnrecorded claims
        to read the tentative state but actual reads the committed state as
        a result we have to commit here so that tentativelyMergePatches
        does the right thing.
   988  I doubt this is right, e.g. withContext should be inherited
  1091  <no description>
 
src/Darcs/UI/Commands/Replace.hs
    82  this heuristic is ham-fisted and silly. Can we drop it? [fixme]
 
src/Darcs/UI/Commands/Unrecord.hs
   321  we may need to honour --ignore-times here, although this command
        does not take that option (yet) [fixme]
 
src/Darcs/UI/CommandsAux.hs
    65  print patch(es) NOTE: should use safe Doc printer, this can be evil
        chars
 
src/Darcs/UI/Email.hs
   134  is this doing mime encoding??
 
src/Darcs/UI/Flags.hs
    20  we want to stop exporting the constructors of DarcsFlag from here.
        First need to change all the relevant code over to the using
        helpers from this module instead.
   344  why filter out null paths from the input? why here and not in
        'maybeFixSubPaths'?
   467  mention the following also: * sendmail(8) is not sendmail-specific;
        * nowadays, desktops often have no MTA or an unconfigured MTA --
        which is awful, because it accepts mail but doesn't relay it; * in
        this case, can be a sendmail(8)-emulating wrapper on top of an MUA
        that sends mail directly to a smarthost; and * on a multi-user
        system without an MTA and on which you haven't got root, can be
        msmtp. [fixme]
 
src/Darcs/UI/Options/All.hs
   391  these options interact (no pun intended) in complex ways that are
        very hard to figure out for users as well as maintainers. I think
        the only solution here is a more radical (and probably
        incompatible) re-design involving all interactivity related
        options. That is beyond the goals of this sub-project (which is
        already large enough).
   578  fix non-default behavior
   833  do something about the nonsensical case (False, Just s)
   874  turn these two into a combined option
 
src/Darcs/UI/PatchHeader.hs
   166  make sure encoding used for logf is the same everywhere probably
        should be locale because the editor will assume it
 
src/Darcs/UI/RunCommand.hs
    96  This code is highly order-dependent because of hidden state: the
        current directory. Like almost all Repository functions, getGlobal
        and getPreflist assume that the cwd is the base of our work repo
        (if any). This is supposed to be ensured by commandPrereq. Which
        means we must first call commandPrereq, then getGlobal and
        getPreflist, and then we must use the (saved) original working
        directory to resolve possibly relative paths to absolute paths.
        [fixme]
 
src/Darcs/UI/SelectChanges.hs
  1067  this code is completely unreadable [fixme]
 
src/Darcs/Util/ByteString.hs
   159  if it is safe to use the expanded definition of isSpaceWord8
        provided by Data.ByteString.Char8, then all this can go.
   230  replace breakFirstPS and breakLastPS with definitions based on
        ByteString's break/breakEnd
   244  rename
   275  rename this function.
 
src/Darcs/Util/Encoding/IConv.hsc
   119  This may not work on platforms where iconv_t is not a pointer.
   154  maybe a better algorithm for increasing the buffer size? and also
        maybe a different starting buffer size?
   166  is this efficient enough?
 
src/Darcs/Util/Index.hs
   303  MAGIC [fixme]
   397  MAGIC [fixme]
   458  calling getFileStatus here is both slightly inefficient and
        slightly race-prone
 
src/Darcs/Util/IsoDate.hs
   839  ; this seems like a terrible idea! it seems like we should get rid
        of it if at all possible, maybe adding an invertDiff function
        [fixme]
 
src/Darcs/Util/Printer.hs
   227  this is a rather ad-hoc hack that further complicates some already
        confusing code. We should find a more general solution. See the
        discussion on issue1639. | Used when rendering a 'Doc' to indicate
        if the result should be encoded to the current locale or left
        alone. In practice this only affects output when a relevant
        DARCS_DONT_ESCAPE_XXX option is set (see Darcs.Util.Printer.Color)
        If in doubt, choose 'Standard'.
   326  try to find another way to do this, it's rather a violation of the
        Doc abstraction
   334  try to find another way to do this, it's rather a violation of the
        Doc abstraction
 
src/Darcs/Util/Ssh.hs
   239  why do we disable progress reporting here?
 
src/Darcs/Util/URL.hs
    49  This whole module should be re-written using a regex matching
        library! The way we do this here is error-prone and inefficient.
 
tests/filepath.sh
    45  This test does not seem to make much sense --repodir is not
        recursive [fixme]
 
tests/git_quoted_filenames.sh
    72  check this for testing on windows.
 
tests/issue595_get_permissions.sh
    33  we avoid this test on Solaris because it seems we can't create
        anything in tmp_restrictive/liberal
 
tests/issue1739-escape-multibyte-chars-correctly.sh
    28  get this working on Windows Seems to be a real bug - darcs doesn't
        record a patch with the right characters.
 
tests/rebase-apply.sh
    52  figure out behaviour of --all and test it (should it answer 'yes'
        to both pulling and suspending?
 
tests/rebase-pull.sh
    51  figure out behaviour of --all and test it (should it answer 'yes'
        to both pulling and suspending?
 
tests/utf8-display.sh
    29  make this work
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20160908/a1eb9061/attachment.asc>