Skip to content

Conversation

@Endilll
Copy link
Contributor

@Endilll Endilll commented Mar 19, 2025

Aaron reported that make_cxx_dr_status replaces all newlines in cxx_dr_status.html, which makes for a huge diff. On Windows, we can't be compatible with all autocrlf modes at once, so this patch adds autodetection of newline style using the existing file, if one is present (which should be the case for all reasonable use cases).

Aaron reported that `make_cxx_dr_status` replaces all newlines in `cxx_dr_status.html`, which makes for a huge diff. On Windows, we can't be compatible with all `autocrlf` modes at once, so this patch adds autodetection of newline style using the existing file, if one is present (which should be the case for all reasonable use cases).
@Endilll Endilll added the clang Clang issues not falling into any other category label Mar 19, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 19, 2025

@llvm/pr-subscribers-clang

Author: Vlad Serebrennikov (Endilll)

Changes

Aaron reported that make_cxx_dr_status replaces all newlines in cxx_dr_status.html, which makes for a huge diff. On Windows, we can't be compatible with all autocrlf modes at once, so this patch adds autodetection of newline style using the existing file, if one is present (which should be the case for all reasonable use cases).


Full diff: https://github.com/llvm/llvm-project/pull/132045.diff

1 Files Affected:

  • (modified) clang/www/make_cxx_dr_status (+17-1)
diff --git a/clang/www/make_cxx_dr_status b/clang/www/make_cxx_dr_status
index c787bbaff0a36..a4b87b32fc5d8 100755
--- a/clang/www/make_cxx_dr_status
+++ b/clang/www/make_cxx_dr_status
@@ -318,6 +318,22 @@ out_html.append('''\
 </html>
 ''')
 
-out_file = open(output, 'w')
+# Make an effort to remain consistent with the existing file.
+# We can't pick one newline style and use it on Windows,
+# because we can't be compatible with all 'autocrlf' modes at once.
+def detect_newline_style(file_path):
+  if not os.path.exists(file_path):
+    return '\n'
+  f = open(file_path)
+  f.readline()
+  if f.newlines is None:
+    return '\n'
+  if isinstance(f.newlines, str):
+    return f.newlines
+  newline = f.newlines[0]
+  print(f"Existing '{file_path}' has inconsistent newlines; picking '{newline.encode('unicode_escape').decode('utf-8')}'")
+  return newline
+    
+out_file = open(output, 'w', newline=detect_newline_style(output))
 out_file.write(''.join(out_html))
 out_file.close()

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM, this works well!

@Endilll Endilll changed the title [clang] Auto-detect which newline style to use for cxx_dr_status.html [clang] Auto-detect which newline style to use for cxx_dr_status.html Mar 19, 2025
@Endilll Endilll merged commit bdca412 into llvm:main Mar 19, 2025
13 checks passed
@Endilll Endilll deleted the newlines-in-dr-status branch March 19, 2025 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants