Skip to content

Add visual line break marker toggle#2133

Open
Elitex07 wants to merge 4 commits into
Acode-Foundation:mainfrom
Elitex07:feature/line-break-marker
Open

Add visual line break marker toggle#2133
Elitex07 wants to merge 4 commits into
Acode-Foundation:mainfrom
Elitex07:feature/line-break-marker

Conversation

@Elitex07
Copy link
Copy Markdown

This pull request introduces a new feature that allows users to display visual markers for line breaks in the editor when text wrapping is enabled. It also includes updates to the development container configuration and Gradle build toolchain settings. The most significant changes are grouped below:

Editor Feature: Line Break Marker

  • Added a new CodeMirror plugin (lineBreakMarker) in src/cm/lineBreakMarker.ts that visually marks line breaks with a "¬" character.
  • Integrated the line break marker feature into the editor manager (src/lib/editorManager.js), including compartment management, settings application, and event handling for toggling the marker. [1] [2] [3] [4]
  • Added a new setting showLineBreakMarker to the default editor settings and the editor settings UI, allowing users to enable or disable the feature. [1] [2]
  • Updated the English language strings to include the new setting label.

Development Environment and Build Tooling

  • Added .devcontainer/devcontainer-lock.json to lock Node.js and Android SDK versions for the development container.
  • Added gradle/gradle-daemon-jvm.properties and updated settings.gradle to configure Gradle toolchains using the Foojay resolver convention plugin. [1] [2]
  • Added local.properties for local Android SDK configuration (note: this file should not be version controlled).

@github-actions github-actions Bot added enhancement New feature or request translations Anything related to Translations Whether a Issue or PR labels May 25, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 25, 2026

Greptile Summary

This PR adds a visual line-break marker feature (¬ symbol) for CodeMirror when text wrapping is enabled, wires it into the editor settings UI, and bundles several unrelated dev-environment changes (devcontainer lock, Gradle toolchain, .gitignore improvements).

  • Line-break marker plugin (src/cm/lineBreakMarker.ts): new ViewPlugin that places a ¬ widget at each non-final line ending; activated only when both showLineBreakMarker and textWrap are on, using a dual-key Compartment in editorManager.js that correctly reacts to changes in either setting.
  • Settings integration: default value (false) added to settings.js, toggle added to editorSettings.js, and the label key added to en-us.json; the info-text i18n key is missing from the language file and falls back to a hardcoded string.
  • local.properties committed: this file contains a machine-specific Windows Android SDK path and should be removed from the PR — it is now also (correctly) listed in .gitignore, but the committed copy will overwrite other developers' local configuration on checkout.

Confidence Score: 3/5

Not safe to merge as-is — a developer-specific local file must be removed before this lands.

The local.properties file (containing a personal Windows Android SDK path) is actively committed in this PR despite the file's own header explicitly warning against it. Any developer pulling this branch would have their local Android build configuration overwritten with a path that only exists on the author's machine. The editor feature itself is implemented cleanly, but the accidental file inclusion needs to be resolved first.

local.properties must be removed from the PR. src/lang/en-us.json should also receive the missing info-text key for the new setting.

Important Files Changed

Filename Overview
local.properties Machine-specific Android SDK path committed despite the file's own header warning against it; will break other developers' builds.
src/cm/lineBreakMarker.ts New ViewPlugin that renders "¬" at each non-final line end; correctly scoped to visible ranges; minor any[] typing issue.
src/lib/editorManager.js Adds compartment and dual-key config for lineBreakMarkerCompartment; correctly piggybacks on the existing update:textWrap listener so the marker hides when wrapping is disabled.
src/settings/editorSettings.js Adds showLineBreakMarker toggle UI; info text key is missing from en-us.json, so the fallback hardcoded string is always used.
src/lang/en-us.json Adds label key "show line break marker" but omits the info-text key "settings-info-editor-show-line-break-marker" used by editorSettings.js.
.gitignore Correctly adds /local.properties to the ignore list, but the file itself is still committed in this PR.
.devcontainer/devcontainer-lock.json Locks devcontainer Node.js and Android SDK feature versions — routine devcontainer hygiene.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User toggles showLineBreakMarker\nor textWrap in Settings] --> B[appSettings fires\nupdate:showLineBreakMarker\nor update:textWrap]
    B --> C[applyOptions called\nwith matching key]
    C --> D{showLineBreakMarker\nAND textWrap?}
    D -- Yes --> E[lineBreakMarkerCompartment\nreconfigured with\nlineBreakMarker plugin]
    D -- No --> F[lineBreakMarkerCompartment\nreconfigured with empty array]
    E --> G[ViewPlugin renders ¬\nat each non-final line.to\nwithin visible ranges]
    F --> H[No markers rendered]
Loading

Comments Outside Diff (1)

  1. src/settings/editorSettings.js, line 858-860 (link)

    P2 Info string key not added to the language file

    strings["settings-info-editor-show-line-break-marker"] is not present in src/lang/en-us.json, so the || "..." fallback is always used for the tooltip/info text. The companion label key "show line break marker" was correctly added, but the info-text key was omitted. This is inconsistent with similar settings like showSpaces, where both the label and the info key live in the language file, and would prevent translators from localising the info text.

Reviews (1): Last reviewed commit: "feat: add visual line break marker for t..." | Re-trigger Greptile

Comment thread local.properties
Comment on lines +1 to +8
## This file must *NOT* be checked into Version Control Systems,
# as it contains information specific to your local configuration.
#
# Location of the SDK. This is only used by Gradle.
# For customization when using a Version Control System, please read the
# header note.
#Mon May 25 14:58:09 IST 2026
sdk.dir=C\:\\Users\\jainy\\AppData\\Local\\Android\\Sdk
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Developer-specific file committed to version control

This file contains a machine-specific Windows SDK path (sdk.dir=C:\Users\jainy\...) and the file header itself says it "must NOT be checked into Version Control Systems." Even though /local.properties was correctly added to .gitignore in this PR, the file is still being committed. Any developer who checks out this branch will have their own local.properties overwritten with a path that doesn't exist on their machine, breaking their Android build setup.

Comment thread src/cm/lineBreakMarker.ts
Comment on lines +29 to +30
getDecorations(view: EditorView): DecorationSet {
let widgets: any[] = [];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 widgets is typed as any[] but the array only ever holds Range<Decoration> values. Using the concrete type improves safety and allows the TypeScript compiler to catch ordering or invalid-range mistakes before runtime.

Suggested change
getDecorations(view: EditorView): DecorationSet {
let widgets: any[] = [];
getDecorations(view: EditorView): DecorationSet {
let widgets: import("@codemirror/state").Range<Decoration>[] = [];

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request translations Anything related to Translations Whether a Issue or PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant