Skip to content
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

rewrite file only when file data changed #613

Merged
merged 1 commit into from
Feb 19, 2015
Merged

rewrite file only when file data changed #613

merged 1 commit into from
Feb 19, 2015

Conversation

haoxins
Copy link
Contributor

@haoxins haoxins commented Feb 1, 2015

also fix #597

@bitwiseman
Copy link
Member

Good start, thanks! Now make it pass tests and also implement it python.

@haoxins
Copy link
Contributor Author

haoxins commented Feb 2, 2015

ok. soon

@haoxins
Copy link
Contributor Author

haoxins commented Feb 9, 2015

also implement it python.

done

test seems failed on master too.

js-beautify output for /tmp/js-beautify-mkdir/example1-default.js was expected to be different

run the cli directly is ok, file created in /tmp/js-beautify-mkdir

@bitwiseman
Copy link
Member

No, the problem is the test expects even unchanged files to be written out...

logToStdout('beautified ' + path.relative(process.cwd(), outfile), config);
} catch (ex) {
onOutputError(ex);
if (code !== pretty) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the problem. You can't just not write the code if it is the same. You need to check (around line 267) if the input and output path are the same. If so, you can check here if the input and output code is unchanged and not change that file. If the input path is empty, output path is empty (for piping cases), or if the input path differs from the output path you still have to write the output, even if it is unchanged.

bitwiseman added a commit that referenced this pull request Feb 19, 2015
@bitwiseman bitwiseman merged commit ccf88f8 into beautifier:master Feb 19, 2015
@bitwiseman
Copy link
Member

Cool. We don't have a test to show that unchanged files are not written, but I can do that. And all the tests pass. Thanks!

bitwiseman added a commit that referenced this pull request Feb 19, 2015
Related to #597 and #613.

While testing, python was missing -r option and also not running some tests.
@haoxins
Copy link
Contributor Author

haoxins commented Feb 20, 2015

😄

@haoxins haoxins deleted the rewrite-only-when-file-changed branch February 20, 2015 10:44
@blino
Copy link

blino commented Oct 15, 2015

The new tests seem to be OSX-specific, touch -A does not work with touch from GNU coreutils (Linux).

@bitwiseman
Copy link
Member

@blino - please open a new issue, and if you are willing to contribute a fix that would be even better!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

only print the file names of changed files
3 participants