-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Address file.managed with contents_newline flag #54929
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
3bcf314
Allow windows to handle binary file in file.managed and add test
d028c8f
Merge branch 'master' into contents_newline_new_fix
xeacott aa439da
Merge branch 'master' into contents_newline_new_fix
xeacott 2e469fe
Merge branch 'master' into contents_newline_new_fix
xeacott c50b416
Fix logic and add more tests
a84896b
Merge branch 'contents_newline_new_fix' of github.com:xeacott/salt in…
8224d61
Merge branch 'master' into contents_newline_new_fix
xeacott c7aea41
Merge branch 'master' into contents_newline_new_fix
xeacott ccbaae2
Merge branch 'master' into contents_newline_new_fix
xeacott 41a9360
Merge branch 'master' into contents_newline_new_fix
xeacott 3f9dfd6
Merge branch 'master' into contents_newline_new_fix
xeacott File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be appropriate to open this in
rb
mode instead? 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, you aren't wrong! I must have forgotten the
b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually no I believe it should remain as r. I thought about this and the PR was incorrectly named (definitely my fault) but opening the file in binary mode and then reading it will produce different results based on the platform, i.e. Window's will read the contents differently than a linux platform. So I decided to just open the file in r as it's more in-line with the change in logic itself. Does that make sense?