New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nimpretty shouldn't change file modif time if no changes => use os.updateFile #9499

Closed
timotheecour opened this Issue Oct 25, 2018 · 0 comments

Comments

Projects
None yet
1 participant
@timotheecour
Copy link
Collaborator

timotheecour commented Oct 25, 2018

suppose tests/nim/nimpretty/t02.nim is a file that's already nimpretty'fied; running nimpretty on it shouldn't change it's file modification time; this is useful for makefile-like tools that rely on file modification time for optimizing (avoid redoing work) on certain operations
currently file modfication time changes:

$ stat tests/nim/nimpretty/t02.nim
16777221 8642335623 -rw-r--r-- 1 timothee staff 0 100 "Oct 24 17:04:54 2018" "Oct 24 17:04:20 2018" "Oct 24 17:04:20 2018" "Oct 16 13:13:23 2018" 4096 8 0 tests/nim/nimpretty/t02.nim
$ $nimc_D/bin/nimpretty --backup:off tests/nim/nimpretty/t02.nim
$ stat tests/nim/nimpretty/t02.nim
16777221 8642335623 -rw-r--r-- 1 timothee staff 0 100 "Oct 24 17:05:07 2018" "Oct 24 17:05:04 2018" "Oct 24 17:05:04 2018" "Oct 16 13:13:23 2018" 4096 8 0 tests/nim/nimpretty/t02.nim

proposal

basically the same motivation as proposed here in this post for D by Andrei Alexandrescu: https://www.mail-archive.com/digitalmars-d@puremagic.com/msg212092.html

# maybe in os.nim:
proc updateFile(filename: string, content: string): bool =
  ## if `filename` already has contents of `content`, return false and don't update `filename` (in particular modification time etc); else update it and return true
  if fileExists(filename):
    if readFile(filename) == content: # TODO: this could be done more efficiently in subsequent PR's without allocation, using a buffer and lower level operations
      return false 
  writeFile(filename, content)
  return true

use cases

then use this in places like nimpretty instead of writeFile

other tools can benefit (eg things like nimfix)

in some cases this can also be a speed benefit (eg writing to disk is typically slower than reading)

more friendly for editors

  • files that are opened in your editor (eg sublime text) currently "flicker" after nimpretty updated them (even if they haven't changed)
  • if you're editing a file (with unsaved changes), running nimpretty will currently cause (in sublime) : Has changed on disk. Do you want to reload it? even if the nimpretty is a noop; using updateFile would avoid this

@timotheecour timotheecour changed the title nimpretty shouldn't change file modif time if no changes nimpretty shouldn't change file modif time if no changes => use os. updateFile Oct 25, 2018

@timotheecour timotheecour changed the title nimpretty shouldn't change file modif time if no changes => use os. updateFile nimpretty shouldn't change file modif time if no changes => use os.updateFile Oct 25, 2018

@Araq Araq closed this in 5fd2827 Oct 25, 2018

narimiran added a commit to narimiran/Nim that referenced this issue Oct 31, 2018

narimiran added a commit to narimiran/Nim that referenced this issue Nov 1, 2018

narimiran added a commit that referenced this issue Nov 1, 2018

narimiran added a commit that referenced this issue Nov 1, 2018

nimpretty: fixes #9499
(cherry picked from commit 5fd2827)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment