darcs

Patch 1468 improve output of darcs record with file arguments

Title improve output of darcs record with file arguments
Superseder Nosy List bfrk
Related Issues
Status accepted Assigned To bfrk
Milestone 2.12.0

Created on 2016-03-16.14:33:57 by bfrk, last changed 2016-04-11.21:21:36 by bfrk.

Files
File name Status Uploaded Type Edit Remove
announcefiles-only-if-verbosity-__-quiet.dpatch bfrk, 2016-03-16.20:25:50 application/x-darcs-patch
patch-preview.txt bfrk, 2016-03-16.14:33:57 text/x-darcs-patch
patch-preview.txt bfrk, 2016-03-16.20:25:50 text/x-darcs-patch
refactored-some_-added-readunrecordedfiltered-and-mayberestrictsubpaths.dpatch bfrk, 2016-03-16.14:33:57 application/x-darcs-patch
unnamed bfrk, 2016-03-16.14:33:57 text/plain
unnamed bfrk, 2016-03-16.20:25:50 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg19087 (view) Author: bfrk Date: 2016-03-16.14:33:57
I think I have sorted out the problems with the mail setup and with my
duplicate personalities in the tracker. Perhaps the automatic screening with
signed patches works now, too.

10 patches for repository http://darcs.net/screened:

patch 2cdbff2f1e2bed298b6838c5cdb2461da28bcfb1
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 15 11:50:24 CET 2016
  * refactored some, added readUnrecordedFiltered and maybeRestrictSubpaths

patch 90e845e3a21c170f86ffff4f88457f44d70edd04
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Wed Mar  9 21:21:07 CET 2016
  * announceFiles only if verbosity /= Quiet

patch 27f1efaed6595f21680b3a127edc26f39d004b3e
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Wed Mar  9 21:51:10 CET 2016
  * added missing hsep function to D.Util.Printer

patch 21563b57f2e32ba237e97b03cb03b9d894a0a67e
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Sun Mar 13 01:18:09 CET 2016
  * add Darcs.Util.Printer.quoted and Darcs.Util.Text.pathlist
  
  The first is like Darcs.Util.Text.quote but returns a Doc. The
  implementation moved from Darcs.Util.Text to Darcs.Util.Printer to avoid a
  circular import.
  The second is for printing space separated lists of quoted file paths,
  something that often appears in user messages.

patch f3086626efaa47c25f63bcb3513e6ecf44dfc59b
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Sat Mar 12 22:49:57 CET 2016
  * added missing Eq and Show instances for ScanKnown

patch 3931b5e0a77b48a751e75cea37d282fea9257052
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Sat Mar 12 22:56:27 CET 2016
  * add new type IncludeBoring for includeBoring option (was Bool)

patch baf20655ce16974637f6097b039fa18283dfa343
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Sun Mar 13 01:12:38 CET 2016
  * added Darcs.Util.Printer.ePutDocLn
  
  The point here is to remove penalties (extra imports of Rendermode, stderr,
  hPutDocLn, two extra parameters) for printing to stderr.

patch d3ef95e9a33c196cfcfb6b86ca379095d328bbab
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Thu Mar 10 21:40:49 CET 2016
  * several fixes and refactorings in fixSubPaths and maybeFixSubPaths
  
  Using Printer/Doc for formatting solves file name encoding problems. Push
  withCurrentDirectory calls down to where they are needed. Improved the
  documentation for these functions.

patch 60cb115141d4b22ea49b8dbea21056090614c91e
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 15 18:20:33 CET 2016
  * resolve issue2494: output of darcs record with file arguments

patch 3e228c0ece18dfa67c2456054157778c1ad95dfc
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 15 18:27:14 CET 2016
  * accept issue2494: output of darcs record with file arguments
  
  This patch also makes some small adaptions to tests/record-misc.sh.
Attachments
msg19089 (view) Author: bfrk Date: 2016-03-16.20:25:51
Updated bundle with one less patch less that wasn't essential. Also this
time actually used --sign. So, unless I botched something else, this
one should really be auto-screened.

10 patches for repository http://darcs.net/screened:

patch 90e845e3a21c170f86ffff4f88457f44d70edd04
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Wed Mar  9 21:21:07 CET 2016
  * announceFiles only if verbosity /= Quiet

patch 3931b5e0a77b48a751e75cea37d282fea9257052
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Sat Mar 12 22:56:27 CET 2016
  * add new type IncludeBoring for includeBoring option (was Bool)

patch baf20655ce16974637f6097b039fa18283dfa343
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Sun Mar 13 01:12:38 CET 2016
  * added Darcs.Util.Printer.ePutDocLn
  
  The point here is to remove penalties (extra imports of Rendermode, stderr,
  hPutDocLn, two extra parameters) for printing to stderr.

patch f3086626efaa47c25f63bcb3513e6ecf44dfc59b
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Sat Mar 12 22:49:57 CET 2016
  * added missing Eq and Show instances for ScanKnown

patch 27f1efaed6595f21680b3a127edc26f39d004b3e
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Wed Mar  9 21:51:10 CET 2016
  * added missing hsep function to D.Util.Printer

patch 21563b57f2e32ba237e97b03cb03b9d894a0a67e
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Sun Mar 13 01:18:09 CET 2016
  * add Darcs.Util.Printer.quoted and Darcs.Util.Text.pathlist
  
  The first is like Darcs.Util.Text.quote but returns a Doc. The
  implementation moved from Darcs.Util.Text to Darcs.Util.Printer to avoid a
  circular import.
  The second is for printing space separated lists of quoted file paths,
  something that often appears in user messages.

patch d3ef95e9a33c196cfcfb6b86ca379095d328bbab
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Thu Mar 10 21:40:49 CET 2016
  * several fixes and refactorings in fixSubPaths and maybeFixSubPaths
  
  Using Printer/Doc for formatting solves file name encoding problems. Push
  withCurrentDirectory calls down to where they are needed. Improved the
  documentation for these functions.

patch 2cdbff2f1e2bed298b6838c5cdb2461da28bcfb1
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 15 11:50:24 CET 2016
  * refactored some, added readUnrecordedFiltered and maybeRestrictSubpaths

patch 60cb115141d4b22ea49b8dbea21056090614c91e
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 15 18:20:33 CET 2016
  * resolve issue2494: output of darcs record with file arguments

patch 3e228c0ece18dfa67c2456054157778c1ad95dfc
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 15 18:27:14 CET 2016
  * accept issue2494: output of darcs record with file arguments
  
  This patch also makes some small adaptions to tests/record-misc.sh.
Attachments
msg19090 (view) Author: bfrk Date: 2016-03-17.07:59:21
Screening this bundle since nobody protested...
msg19107 (view) Author: gh Date: 2016-03-18.21:02:07
Does this bundle also fix http://bugs.darcs.net/issue2211 ?

(Sorry I don't have time righ now to check.)
msg19120 (view) Author: bfrk Date: 2016-03-29.22:33:52
Am 18.03.2016 um 22:02 schrieb Guillaume Hoffmann:
>
> Guillaume Hoffmann <guillaumh@gmail.com> added the comment:
>
> Does this bundle also fix http://bugs.darcs.net/issue2211 ?

Yes, it does.

Cheers
Ben
msg19126 (view) Author: gh Date: 2016-04-01.19:11:48
* announceFiles only if verbosity /= Quiet: OK
* add new type IncludeBoring for includeBoring option (was Bool): OK
* added missing Eq and Show instances for ScanKnown: OK

* added missing hsep function to D.Util.Printer: OK
* added Darcs.Util.Printer.ePutDocLn: OK
* add Darcs.Util.Printer.quoted and Darcs.Util.Text.pathlist: OK

* several fixes and refactorings in fixSubPaths and maybeFixSubPaths:
Good, although pushing "withDirectory" deeper does not seem like a win
to me (multiple calls to cwd instead of 1)

* refactored some, added readUnrecordedFiltered and
maybeRestrictSubpaths: Good, although it seems weird to have a patch
that just introduces a function with no use of it yet, *but* I'm
actually using it in bundle http://bugs.darcs.net/patch1474 .

* "accept issue2494: output of darcs record with file arguments" and
"resolve issue2494":


This is good for the "darcs record" but I think "record" and "whatsnew"
should be coherent. Currently there are differences between both. With
"whatsnew", non-repository paths are properly reported, but not-added
and non-existing paths are not reported.

Can you change the shell test and whatsnew module accordingly?
msg19131 (view) Author: gh Date: 2016-04-03.21:29:32
An extra request (only if it's easy): can you see whether you can also
fix http://bugs.darcs.net/issue1481
msg19143 (view) Author: bfrk Date: 2016-04-05.21:07:01
> pushing "withDirectory" deeper does not seem like a win
> to me (multiple calls to cwd instead of 1)

I think you are mistaken here. The number of calls to cwd is the same as
before, I just do it at the latest possible moment i.e. right before
ioAbsolute is called. The IO actions in between are independent of the
cwd, thus separating the change of the cwd from the place where it is
needed just obscures things.

> This is good for the "darcs record" but I think "record" and 
> "whatsnew" should be coherent. Currently there are differences
> between both. With "whatsnew", non-repository paths are properly
> reported, but not-added and non-existing paths are not reported.
>
> Can you change the shell test and whatsnew module accordingly?

Yes, I am all for consistency in the UI. Will try to find a time slot to
work on this.

> can you see whether you can also
> fix http://bugs.darcs.net/issue1481

I will try. This is probably not easy, but I am inclined to work out how
to make it work. This is a hairy problem because we have to calculate
the file renames backwards. I am not even sure yet how to specify
exactly what should happen when you give path arguments in the face of
pending renames. I have also thought a lot about this when I worked on
the mark-conflicts command.

BTW thanks for digging out all these related issues!
msg19157 (view) Author: gh Date: 2016-04-11.17:38:43
OK I agree that using "withCurrentDirectory" close to "ioAbsolute" is
desirable because more understandable. I think there *are* more calls to
cwd after the aforementioned patch because fixit is called several
times, but it's not that important.

To avoid blocking other bundles, I'm accepting this one and creating
http://bugs.darcs.net/issue2496 as a reminder for making whatsnew's
output similar to record.
msg19162 (view) Author: bfrk Date: 2016-04-11.21:21:36
You have been right all along, as usual. Sorry for being thick, how
could I not see this?

Anyway, I guess now it would make sense to factor

  \r p -> withCurrentDirectory r $ ioAbsolute p

into its own function (somewhere under Darcs.Util or perhaps
Darcs.UI.Command.Util).

I also wonder whether what we do here is really the right thing to do.
Interpreting the argument relative to the (original) cwd is of course
correct. But I am less sure it meets user's expectations that we fall
back to interpreting it relative to the repo path; that is, unless the
user explicitly gave a --repodir option; to cover this case we should
perhaps pass this option to the function and do the alternative
interpretation only if the option is present?
History
Date User Action Args
2016-03-16 14:33:57bfrkcreate
2016-03-16 20:25:51bfrksetfiles: + patch-preview.txt, announcefiles-only-if-verbosity-__-quiet.dpatch, unnamed
messages: + msg19089
2016-03-17 07:59:21bfrksetstatus: needs-screening -> needs-review
messages: + msg19090
2016-03-18 21:02:07ghsetmessages: + msg19107
2016-03-29 22:33:53bfrksetmessages: + msg19120
2016-04-01 19:11:49ghsetstatus: needs-review -> followup-requested
assignedto: bfrk
messages: + msg19126
2016-04-03 21:29:33ghsetmessages: + msg19131
2016-04-05 21:07:02bfrksetmessages: + msg19143
2016-04-11 17:38:43ghsetstatus: followup-requested -> accepted
messages: + msg19157
milestone: 2.12.0
2016-04-11 21:21:36bfrksetmessages: + msg19162