darcs

Patch 1772 remove currentDirectory from TreeMonad

Title remove currentDirectory from TreeMonad
Superseder Nosy List bfrk
Related Issues
Status obsoleted Assigned To
Milestone

Created on 2018-12-03.13:36:16 by bfrk, last changed 2021-01-19.13:33:17 by bfrk.

Files
File name Status Uploaded Type Edit Remove
patch-preview.txt dead bfrk, 2018-12-03.13:36:16 text/x-darcs-patch
patch-preview.txt dead bfrk, 2019-02-26.07:47:54 text/x-darcs-patch
patch-preview.txt bfrk, 2019-08-29.22:31:41 text/x-darcs-patch
remove-currentdirectory-from-treemonad.dpatch dead bfrk, 2018-12-03.13:36:16 application/x-darcs-patch
remove-currentdirectory-from-treemonad.dpatch dead bfrk, 2019-02-26.07:47:55 application/x-darcs-patch
remove-currentdirectory-from-treemonad.dpatch bfrk, 2019-08-29.22:31:41 application/x-darcs-patch
unnamed dead bfrk, 2018-12-03.13:36:16 text/plain
unnamed dead bfrk, 2019-02-26.07:47:55 text/plain
unnamed bfrk, 2019-08-29.22:31:41 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg20566 (view) Author: bfrk Date: 2018-12-03.13:36:16
1 patch for repository http://darcs.net/screened:

patch 1e1ffcee75dc90ad667197b513b7117e83cfe859
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Dec  3 14:37:26 CET 2018
  * remove currentDirectory from TreeMonad
  
  Adding a notion of "current directory" to the TreeMonad is an unnecessary
  complication at best. It is clearer to explicitly pass the current directory
  when using the TreeMonad interface. Thankfully, this feature was used only
  in a single function in Darcs.UI.Commands.Util.
Attachments
msg20581 (view) Author: bfrk Date: 2018-12-04.18:31:09
I am not screening this one immediately. If you think that
currentDirectory is an important part of TreeMonad, please shout.
msg20591 (view) Author: ganesh Date: 2018-12-06.06:29:48
This change doesn't affect whether darcs does filesystem operations
relative to the current working directory, does it?

A while ago ago we spent some time looking at whether we could stop
darcs from depending on the current working directory for the 
process, as it's implicit global state that makes it hard to use the 
darcs library from multiple threads. One worry was that it would
have a significant performance impact because it's slower for the OS
to look up an absolute path than one relative to CWD. I think a
factor of two was mentioned, but I have no idea what the current
situation would be.

The patch also has a lot of reformatting along with the logic
changes that will make it harder to review.
msg20604 (view) Author: bfrk Date: 2018-12-10.14:16:26
> This change doesn't affect whether darcs does filesystem operations
> relative to the current working directory, does it?

No, of course not. It just means you cannot zoom into a subtree as a
"built-in" operation of TreeMonad.

> A while ago ago we spent some time looking at whether we could stop
> darcs from depending on the current working directory for the 
> process, as it's implicit global state that makes it hard to use the 
> darcs library from multiple threads. One worry was that it would
> have a significant performance impact because it's slower for the OS
> to look up an absolute path than one relative to CWD. I think a
> factor of two was mentioned, but I have no idea what the current
> situation would be.

I guess it also depends on how long the path to the repository is. My
guess is that this is especially bad in Haskell, since all the IO
operations assume FilePath = String.

The problem with the current working directory is the file system API,
which for traditional reasons has only one working directory for the
whole process. A more modern API would allow the user to create multiple
working directories and to pass them explicitly (and interpret relative
paths using explicit working directories). We can emulate such a modern
API but this would be pointless, as it would rob us of the performance
improvement.

However, this is alltogether a different issue.

> The patch also has a lot of reformatting along with the logic
> changes that will make it harder to review.

I has some reformatting, I wouldn't say "a lot". The import list
reformat shouldn't be an issue for review. The existsAnycase function
has changes on almost all its code lines, so I saw no sense in
preserving the existing layout. The data type reformat could have been
left out, though, sorry for that.
msg20658 (view) Author: bfrk Date: 2019-02-26.07:47:55
Amended to resolve a conflict with screened. Also add a second related
patch.

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

patch 8634f8ef8d8db0a3a394d435fdf0f4cfc9da864a
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Dec  3 14:37:26 CET 2018
  * remove currentDirectory from TreeMonad
  
  Adding a notion of "current directory" to the TreeMonad is an unnecessary
  complication at best. It is clearer to explicitly pass the current directory
  when using the TreeMonad interface. Thankfully, this feature was used only
  in a single function in Darcs.UI.Commands.Util.

patch fbc9ed323a712195b44e7b2c86ac31f25d801101
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Feb 26 08:42:27 CET 2019
  * existsAnycase: don't error if subdir is not found
  
  This function queries whether a path exists, disregarding upper/lower case,
  so it makes sense to just return False if a prefix of the path is not found.
Attachments
msg21281 (view) Author: bfrk Date: 2019-08-29.22:31:41
I have separated out or removed all the gratuitious layout changes. This
should be easier to review now.

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

patch fc0252b62b16206b74b6847ed4baae4741d6c5bc
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Dec  3 14:37:26 CET 2018
  * remove currentDirectory from TreeMonad
  
  Adding a notion of "current directory" to the TreeMonad is an unnecessary
  complication at best. It is clearer to explicitly pass the current directory
  when using the TreeMonad interface. Thankfully, this feature was used only
  in a single function in Darcs.UI.Commands.Util.

patch de379eef2e7c552013acadd3f94918149e1add06
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Aug 30 00:32:52 CEST 2019
  * cleanup layout and import lists in Darcs.UI.Commands.Util.Tree

patch c6597e29db19c1ac3c9fd65c878ff671204180b7
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Feb 26 08:42:27 CET 2019
  * existsAnycase: don't error if subdir is not found
  
  This function queries whether a path exists, disregarding upper/lower case,
  so it makes sense to just return False if a prefix of the path is not found.
Attachments
History
Date User Action Args
2018-12-03 13:36:16bfrkcreate
2018-12-04 18:31:09bfrksetmessages: + msg20581
2018-12-06 06:29:49ganeshsetmessages: + msg20591
2018-12-10 14:16:27bfrksetmessages: + msg20604
2019-02-26 07:47:55bfrksetfiles: + patch-preview.txt, remove-currentdirectory-from-treemonad.dpatch, unnamed
messages: + msg20658
2019-06-15 14:13:52bfrksetstatus: needs-screening -> in-discussion
2019-08-29 22:31:41bfrksetfiles: + patch-preview.txt, remove-currentdirectory-from-treemonad.dpatch, unnamed
messages: + msg21281
2021-01-19 13:33:17bfrksetstatus: in-discussion -> obsoleted