From f3754c922d88b9aab2dfb7c919634afbc060dda6 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Fri, 17 Jul 2026 05:03:52 +0200 Subject: [PATCH 1/2] Improve documentation of line ending handling --- CHANGELOG.md | 3 ++- src/solidlsp/ls_utils.py | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a001291c..e8e3ac1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,8 @@ Status of the `main` branch. Changes prior to the next official version change w - During project creation, language composition percentages are now computed relative to the total number of recognised source files instead of all files, i.e. unrecognised files are ignored in the percentage computation. - - Fix: Use LSP-compliant line splitting ("\n", "\r\n" and "\r" can define line breaks); previously only "\n" considered + - Consider all LSP-compliant line endings ("\n", "\r\n" and "\r") in `TextUtils`, noting that + only "\n" appears in files read by `FileUtils.read_file` (Serena's default reading mechanism) - Fix: Apply consistent line splitting across tools, uniformly applying the LSP splitting semantics; affects `search_text` used by `search_for_pattern` tool (reported in #1684) - Fix in `TextUtils` (used by editors): Deleting up to the end of the file, referencing the line one past diff --git a/src/solidlsp/ls_utils.py b/src/solidlsp/ls_utils.py index d95723ad..b87355bd 100644 --- a/src/solidlsp/ls_utils.py +++ b/src/solidlsp/ls_utils.py @@ -32,6 +32,13 @@ class InvalidTextLocationError(Exception): class TextStepper: + r""" + A utility class for stepping through a text string line by line, keeping track of the current line and column numbers. + + It handles the newline sequences "\n", "\r\n", and "\r" as defined by the Language Server Protocol (LSP). + However, note that files read through `FileUtils.read_file` will contain only "\n" (LF). + """ + def __init__(self, chars: str): self._chars = chars self._len = len(chars) From e8a550763b7e28ff434f042806bd7f13d1e53eae Mon Sep 17 00:00:00 2001 From: Ehsan Date: Fri, 17 Jul 2026 06:11:26 +0300 Subject: [PATCH 2/2] fix: normalize line endings in read_file's charset_normalizer fallback (#1706) FileUtils.read_file has two return paths with different newline semantics. The primary path, open(file_path, encoding=encoding).read(), uses the default newline=None and therefore applies universal newlines, so CR never reaches callers. The charset_normalizer fallback decodes the raw bytes instead (match.raw.decode(match.encoding)), which performs no newline translation, so any file that fails to decode with the project's configured encoding re-enters Serena carrying its CR characters. Normalize to LF at the fallback's return, matching the primary path. --- CHANGELOG.md | 8 ++++ src/solidlsp/ls_utils.py | 8 +++- test/solidlsp/util/test_ls_utils.py | 64 +++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8e3ac1d..4f5d9d95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ Status of the `main` branch. Changes prior to the next official version change will appear here. +* General: + - Fix: `FileUtils.read_file`'s `charset_normalizer` fallback (used when a file cannot be decoded with + the project's configured `encoding`) decoded the raw bytes directly and therefore skipped the + universal-newline translation that the primary read path applies. CR characters from disk thus + reached Serena's in-memory file contents, where the rest of the code assumes LF-normalized text and + the `line_ending` setting is meant to be the single point of line-ending translation on write. The + fallback now normalizes line endings to LF, consistently with the primary path. + # v1.6.0 (2026-07-16) * General: diff --git a/src/solidlsp/ls_utils.py b/src/solidlsp/ls_utils.py index b87355bd..7ff1bb71 100644 --- a/src/solidlsp/ls_utils.py +++ b/src/solidlsp/ls_utils.py @@ -387,6 +387,9 @@ class FileUtils: Reads the file at the given path using the given encoding and returns the contents as a string. If decoding fails, tries to detect the encoding using charset_normalizer. + Line endings are normalized to LF (universal newlines), irrespective of the encoding + used to decode the file. + Raises FileNotFoundError if the file does not exist. """ if not os.path.exists(file_path): @@ -403,7 +406,10 @@ class FileUtils: log.warning( f"Could not decode {file_path} with encoding='{encoding}'; using best match '{match.encoding}' instead", ) - return match.raw.decode(match.encoding) + # Decoding the raw bytes bypasses the universal-newline translation that the + # open() call above applies, so normalize explicitly to keep both paths equivalent. + decoded = match.raw.decode(match.encoding) + return decoded.replace("\r\n", "\n").replace("\r", "\n") raise ude except Exception as exc: log.error(f"Failed to read '{file_path}' with encoding '{encoding}': {exc}") diff --git a/test/solidlsp/util/test_ls_utils.py b/test/solidlsp/util/test_ls_utils.py index cad603c3..a4876645 100644 --- a/test/solidlsp/util/test_ls_utils.py +++ b/test/solidlsp/util/test_ls_utils.py @@ -4,6 +4,8 @@ import hashlib from pathlib import Path from unittest.mock import patch +import pytest + from solidlsp.ls_utils import FileUtils @@ -40,3 +42,65 @@ def test_download_file_verified_writes_decoded_response_body(tmp_path: Path) -> ) assert target_path.read_bytes() == payload + + +# A file that cannot be decoded with the project encoding, forcing read_file's +# charset_normalizer fallback. The accented characters make the bytes invalid UTF-8, +# and the content is long enough for encoding detection to be reliable. +_CP1252_LINES = [ + "# -*- coding: cp1252 -*-", + "# Author: José Fernández", + "# Copyright (c) 2019 Müller & Söhne GmbH.", + "", + "import os", + "", + "", + "class ConfiguracionBasica:", + ' """Clase de configuración para el módulo de facturación."""', + "", + " def __init__(self, nombre, valor=None):", + " self.nombre = nombre", + " self.valor = valor", + "", + " def describir(self):", + ' return f"{self.nombre}: {self.valor}"', +] + + +def test_read_file_fallback_normalizes_crlf(tmp_path: Path) -> None: + """The charset_normalizer fallback should apply universal newlines, just like the primary path.""" + file_path = tmp_path / "config_cp1252.py" + file_path.write_bytes(("\r\n".join(_CP1252_LINES) + "\r\n").encode("cp1252")) + + # guard against a vacuous test: the fixture must actually force the fallback + with pytest.raises(UnicodeDecodeError): + file_path.read_text(encoding="utf-8") + + content = FileUtils.read_file(str(file_path), "utf-8") + + assert "José Fernández" in content, "fallback should decode the file as cp1252" + assert "\r" not in content + assert content.splitlines() == _CP1252_LINES + + +def test_read_file_fallback_normalizes_lone_cr(tmp_path: Path) -> None: + """Old-style lone CR separators should be normalized by the fallback as well.""" + file_path = tmp_path / "lone_cr_cp1252.py" + file_path.write_bytes(("\r".join(_CP1252_LINES) + "\r").encode("cp1252")) + + content = FileUtils.read_file(str(file_path), "utf-8") + + assert "\r" not in content + assert content.splitlines() == _CP1252_LINES + + +def test_read_file_primary_path_normalizes_crlf(tmp_path: Path) -> None: + """Control: the primary open() path already normalizes; both paths must agree.""" + lines = ["import os", "", "def f():", " return 1"] + file_path = tmp_path / "config_utf8.py" + file_path.write_bytes(("\r\n".join(lines) + "\r\n").encode("utf-8")) + + content = FileUtils.read_file(str(file_path), "utf-8") + + assert "\r" not in content + assert content.splitlines() == lines