common: util: stop using binary mode in SetContents

verible/common/formatting/basic-format-style-init.cc

@@ -37,6 +37,9 @@ ABSL_FLAG(int, over_column_limit_penalty, 100,
 ABSL_FLAG(int, line_break_penalty, 2,
           "Penalty added to solution for each introduced line break.");
 
+ABSL_FLAG(verible::LineTerminatorStyle, line_terminator,
+          verible::LineTerminatorStyle::kLF, "Line terminator");
+
 namespace verible {
 void InitializeFromFlags(BasicFormatStyle *style) {
 #define STYLE_FROM_FLAG(name) style->name = absl::GetFlag(FLAGS_##name)
@@ -47,6 +50,7 @@ void InitializeFromFlags(BasicFormatStyle *style) {
   STYLE_FROM_FLAG(column_limit);
   STYLE_FROM_FLAG(over_column_limit_penalty);
   STYLE_FROM_FLAG(line_break_penalty);
+  STYLE_FROM_FLAG(line_terminator);
 
 #undef STYLE_FROM_FLAG
 }

verible/common/formatting/basic-format-style.cc

@@ -48,4 +48,39 @@ std::string AbslUnparseFlag(const IndentationStyle &mode) {
   return stream.str();
 }
 
+static const verible::EnumNameMap<LineTerminatorStyle> &
+LineTerminatorStyleStrings() {
+  static const verible::EnumNameMap<LineTerminatorStyle>
+      kLineTerminatorStyleStringMap({
+          {"CRLF", LineTerminatorStyle::kCRLF},
+          {"LF", LineTerminatorStyle::kLF},
+      });
+  return kLineTerminatorStyleStringMap;
+}
+
+void EmitLineTerminator(LineTerminatorStyle style, std::ostream &stream) {
+  switch (style) {
+    case LineTerminatorStyle::kLF:
+      stream << "\n";
+      break;
+    case LineTerminatorStyle::kCRLF:
+      stream << "\r\n";
+      break;
+  }
+}
+
+std::ostream &operator<<(std::ostream &stream, LineTerminatorStyle style) {
+  return LineTerminatorStyleStrings().Unparse(style, stream);
+}
+
+bool AbslParseFlag(std::string_view text, LineTerminatorStyle *mode,
+                   std::string *error) {
+  return LineTerminatorStyleStrings().Parse(text, mode, error,
+                                            "LineTerminatorStyle");
+}
+
+std::string AbslUnparseFlag(const LineTerminatorStyle &mode) {
+  return std::string{LineTerminatorStyleStrings().EnumName(mode)};
+}
+
 }  // namespace verible

verible/common/formatting/basic-format-style.h

@@ -21,6 +21,21 @@
 
 namespace verible {
 
+enum class LineTerminatorStyle {
+  // Line Feed `\n` (UNIX Style)
+  kLF,
+  // Carriage return + Line Feed `\r\n` (DOS Style)
+  kCRLF,
+};
+
+void EmitLineTerminator(LineTerminatorStyle style, std::ostream &stream);
+
+std::ostream &operator<<(std::ostream &stream, LineTerminatorStyle style);
+
+bool AbslParseFlag(std::string_view, LineTerminatorStyle *, std::string *);
+
+std::string AbslUnparseFlag(const LineTerminatorStyle &);
+
 // Style configuration common to all languages.
 struct BasicFormatStyle {
   // Each indentation level adds this many spaces.
@@ -42,6 +57,9 @@ struct BasicFormatStyle {
   // Penalty added to solution for each introduced line break.
   int line_break_penalty = 2;
 
+  // Line terminator character sequence
+  LineTerminatorStyle line_terminator = LineTerminatorStyle::kLF;
+
   // -- Note: when adding new fields, add them in basic_format_style_init.cc
 };
 

verible/verilog/formatting/BUILD

@@ -215,6 +215,7 @@ cc_library(
     srcs = ["comment-controls.cc"],
     hdrs = ["comment-controls.h"],
     deps = [
+        "//verible/common/formatting:basic-format-style",
         "//verible/common/strings:comment-utils",
         "//verible/common/strings:display-utils",
         "//verible/common/strings:line-column-map",
@@ -223,7 +224,6 @@ cc_library(
         "//verible/common/text:token-stream-view",
         "//verible/common/util:logging",
         "//verible/common/util:range",
-        "//verible/common/util:spacer",
         "//verible/verilog/parser:verilog-parser",
         "//verible/verilog/parser:verilog-token-classifications",
         "//verible/verilog/parser:verilog-token-enum",
@@ -236,6 +236,7 @@ cc_test(
     srcs = ["comment-controls_test.cc"],
     deps = [
         ":comment-controls",
+        "//verible/common/formatting:basic-format-style",
         "//verible/common/strings:line-column-map",
         "//verible/common/strings:position",
         "//verible/common/text:token-info-test-util",

verible/verilog/formatting/comment-controls.cc

@@ -23,6 +23,7 @@
 
 #include "absl/strings/str_split.h"
 #include "absl/strings/strip.h"
+#include "verible/common/formatting/basic-format-style.h"
 #include "verible/common/strings/comment-utils.h"
 #include "verible/common/strings/display-utils.h"
 #include "verible/common/strings/line-column-map.h"
@@ -31,7 +32,6 @@
 #include "verible/common/text/token-stream-view.h"
 #include "verible/common/util/logging.h"
 #include "verible/common/util/range.h"
-#include "verible/common/util/spacer.h"
 #include "verible/verilog/parser/verilog-parser.h"
 #include "verible/verilog/parser/verilog-token-classifications.h"
 #include "verible/verilog/parser/verilog-token-enum.h"
@@ -121,7 +121,7 @@ static size_t NewlineCount(std::string_view s) {
 void FormatWhitespaceWithDisabledByteRanges(
     std::string_view text_base, std::string_view space_text,
     const ByteOffsetSet &disabled_ranges, bool include_disabled_ranges,
-    std::ostream &stream) {
+    std::ostream &stream, verible::LineTerminatorStyle line_terminator_style) {
   VLOG(3) << __FUNCTION__;
   CHECK(verible::IsSubRange(space_text, text_base));
   const int start = std::distance(text_base.begin(), space_text.begin());
@@ -136,7 +136,7 @@ void FormatWhitespaceWithDisabledByteRanges(
   if (space_text.empty() && start != 0) {
     if (!disabled_ranges.Contains(start)) {
       VLOG(3) << "output: 1*\"\\n\" (empty space text)";
-      stream << '\n';
+      verible::EmitLineTerminator(line_terminator_style, stream);
       return;
     }
   }
@@ -159,7 +159,9 @@ void FormatWhitespaceWithDisabledByteRanges(
           text_base.substr(range.first, range.second - range.first));
       const size_t newline_count = NewlineCount(enabled);
       VLOG(3) << "output: " << newline_count << "*\"\\n\" (formatted)";
-      stream << verible::Spacer(newline_count, '\n');
+      for (size_t i = 0; i < newline_count; i++) {
+        verible::EmitLineTerminator(line_terminator_style, stream);
+      }
       partially_enabled = true;
       total_enabled_newlines += newline_count;
     }
@@ -177,7 +179,7 @@ void FormatWhitespaceWithDisabledByteRanges(
   // Print at least one newline if some subrange was format-enabled.
   if (partially_enabled && total_enabled_newlines == 0 && start != 0) {
     VLOG(3) << "output: 1*\"\\n\"";
-    stream << '\n';
+    verible::EmitLineTerminator(line_terminator_style, stream);
   }
 }
 

verible/verilog/formatting/comment-controls.h

@@ -18,6 +18,7 @@
 #include <ostream>
 #include <string_view>
 
+#include "verible/common/formatting/basic-format-style.h"
 #include "verible/common/strings/line-column-map.h"
 #include "verible/common/strings/position.h"  // for ByteOffsetSet, LineNumberSet
 #include "verible/common/text/token-stream-view.h"
@@ -48,7 +49,7 @@ verible::ByteOffsetSet EnabledLinesToDisabledByteRanges(
 void FormatWhitespaceWithDisabledByteRanges(
     std::string_view text_base, std::string_view space_text,
     const verible::ByteOffsetSet &disabled_ranges, bool include_disabled_ranges,
-    std::ostream &stream);
+    std::ostream &stream, verible::LineTerminatorStyle line_terminator_style);
 
 }  // namespace formatter
 }  // namespace verilog

verible/verilog/formatting/comment-controls_test.cc

@@ -22,6 +22,7 @@
 #include "absl/strings/str_join.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include "verible/common/formatting/basic-format-style.h"
 #include "verible/common/strings/line-column-map.h"
 #include "verible/common/strings/position.h"
 #include "verible/common/text/token-info-test-util.h"
@@ -328,7 +329,8 @@ TEST(FormatWhitespaceWithDisabledByteRangesTest, InvalidSubstring) {
   const std::string_view foo("foo"), bar("bar");
   std::ostringstream stream;
   EXPECT_DEATH(
-      FormatWhitespaceWithDisabledByteRanges(foo, bar, {}, true, stream),
+      FormatWhitespaceWithDisabledByteRanges(foo, bar, {}, true, stream,
+                                             verible::LineTerminatorStyle::kLF),
       "IsSubRange");
 }
 
@@ -388,7 +390,8 @@ TEST(FormatWhitespaceWithDisabledByteRangesTest, EmptyStrings) {
         test.substring_range.first,
         test.substring_range.second - test.substring_range.first);
     FormatWhitespaceWithDisabledByteRanges(test.full_text, substr,
-                                           test.disabled_ranges, true, stream);
+                                           test.disabled_ranges, true, stream,
+                                           verible::LineTerminatorStyle::kLF);
     EXPECT_EQ(stream.str(), test.expected)
         << "text: \"" << test.full_text << "\", sub: \"" << substr
         << "\", disabled: " << test.disabled_ranges;

verible/verilog/formatting/formatter.cc

@@ -993,7 +993,7 @@ void Formatter::Emit(bool include_disabled, std::ostream &stream) const {
         full_text.substr(position, front_offset - position));
     FormatWhitespaceWithDisabledByteRanges(full_text, leading_whitespace,
                                            disabled_ranges_, include_disabled,
-                                           stream);
+                                           stream, style_.line_terminator);
 
     // When front of first token is format-disabled, the previous call will
     // already cover the space up to the front token, in which case,
@@ -1010,7 +1010,7 @@ void Formatter::Emit(bool include_disabled, std::ostream &stream) const {
   const std::string_view trailing_whitespace(full_text.substr(position));
   FormatWhitespaceWithDisabledByteRanges(full_text, trailing_whitespace,
                                          disabled_ranges_, include_disabled,
-                                         stream);
+                                         stream, style_.line_terminator);
 }
 
 }  // namespace formatter

verible/verilog/parser/verilog.lex

@@ -294,7 +294,13 @@ PragmaEndProtected {Pragma}{Space}+protect{Space}+end_protected
     yymore();
   }
   {LineTerminator} {
-    yyless(yyleng-1);  /* return \n to input stream */
+    // This is needed in order to avoid lexical differences when the line
+    // endings are changed through verible-verilog-format. See [1]
+    // [1] https://github.com/chipsalliance/verible/pull/2371
+    if(yytext[yyleng-2] == '\r')
+      yyless(yyleng-2);  /* return \r\n to input stream */
+    else
+      yyless(yyleng-1);  /* return \n to input stream */
     UpdateLocation();
     yy_pop_state();
     return TK_EOL_COMMENT;

verible/verilog/tools/formatter/BUILD

@@ -132,3 +132,11 @@ sh_test_with_runfiles_lib(
     args = ["$(location :verible-verilog-format)"],
     data = [":verible-verilog-format"],
 )
+
+sh_test_with_runfiles_lib(
+    name = "format-line_terminator_test",
+    size = "small",
+    srcs = ["format_line_terminator_test.sh"],
+    args = ["$(location :verible-verilog-format)"],
+    data = [":verible-verilog-format"],
+)

verible/verilog/tools/formatter/format_line_terminator_test.sh

@@ -0,0 +1,53 @@
+#!/usr/bin/env bash
+# Copyright 2017-2025 The Verible Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+declare -r MY_INPUT_FILE="${TEST_TMPDIR}/myinput.txt"
+declare -r MY_OUTPUT_FILE="${TEST_TMPDIR}/myoutput.txt"
+declare -r MY_EXPECT_FILE="${TEST_TMPDIR}/myexpect.txt"
+
+# Get tool from argument
+[[ "$#" == 1 ]] || {
+  echo "Expecting 1 positional argument, verible-verilog-format path."
+  exit 1
+}
+formatter="$(rlocation ${TEST_WORKSPACE}/${1})"
+
+# Create files with LF and CRLF line terminators. They are properly formatted, so running
+# the formatter should only change line endings
+printf "// some comment\n/* some other comment */\nmodule m;\nendmodule\n" > "${MY_INPUT_FILE}LF"
+printf "// some comment\r\n/* some other comment */\r\nmodule m;\r\nendmodule\r\n" > "${MY_INPUT_FILE}CRLF"
+
+
+for newline in LF CRLF; do
+  cp "${MY_INPUT_FILE}$newline" "${MY_EXPECT_FILE}$newline"
+done
+
+# Test any combination of input line terminators and output line terminators.
+# Test both inline formatting and standard output
+for original_newline in LF CRLF; do
+  for target_newline in LF CRLF; do
+    PROPER_INPUT_FILE="${MY_INPUT_FILE}$original_newline"
+    PROPER_EXPECT_FILE="${MY_EXPECT_FILE}$target_newline"
+
+    ${formatter} --line_terminator=$target_newline $PROPER_INPUT_FILE > ${MY_OUTPUT_FILE}
+    cmp ${MY_OUTPUT_FILE} $PROPER_EXPECT_FILE || exit 1
+
+    cp $PROPER_INPUT_FILE ${MY_OUTPUT_FILE}
+    ${formatter} --line_terminator=$target_newline --inplace ${MY_OUTPUT_FILE}
+    cmp ${MY_OUTPUT_FILE} $PROPER_EXPECT_FILE || exit 2
+  done
+done
+
+echo "PASS"

verible/verilog/tools/formatter/verilog-format.cc

@@ -44,6 +44,11 @@
 #include "verible/verilog/formatting/format-style.h"
 #include "verible/verilog/formatting/formatter.h"
 
+#ifdef _WIN32
+#include <fcntl.h>
+#include <io.h>
+#endif
+
 using absl::StatusCode;
 using verible::LineNumberSet;
 using verilog::formatter::ExecutionControl;
@@ -135,7 +140,7 @@ static bool formatOneFile(std::string_view filename,
                           bool *any_changes) {
   const bool inplace = absl::GetFlag(FLAGS_inplace);
   const bool check_changes_only = absl::GetFlag(FLAGS_verify);
-  const bool is_stdin = filename == "-";
+  const bool is_stdin = verible::file::IsStdin(filename);
   const auto &stdin_name = absl::GetFlag(FLAGS_stdin_name);
 
   if (inplace && is_stdin) {
@@ -245,6 +250,11 @@ static bool formatOneFile(std::string_view filename,
 }
 
 int main(int argc, char **argv) {
+#ifdef _WIN32
+  // Windows messes with newlines by default. Fix this here.
+  _setmode(_fileno(stdout), _O_BINARY);
+#endif
+
   const auto usage = absl::StrCat("usage: ", argv[0],
                                   " [options] <file> [<file...>]\n"
                                   "To pipe from stdin, use '-' as <file>.");