Giter Club home page Giter Club logo

Comments (6)

BetaRays avatar BetaRays commented on June 2, 2024

Apparently this patch fixes it (see the context), but I don’t know why the zero-width spaces were there in the first place.

diff --git a/src/shared/styling.js b/src/shared/styling.js
index bf09a649a..d18e881c3 100644
--- a/src/shared/styling.js
+++ b/src/shared/styling.js
@@ -150,8 +150,7 @@ export function getDirectiveTemplate (d, text, offset, options) {
     if (isQuoteDirective(d)) {
         const newtext = text
             // Don't show the directive itself
-            .replace(/\n>\s/g, '\n\u200B\u200B')
-            .replace(/\n>/g, '\n\u200B')
+            .replace(/\n>\s?/g, '\n')
             .replace(/\n$/, ''); // Trim line-break at the end
         return template(newtext, offset, options);
     } else {

@jcbrand Should I open a merge request for this? Or is this a wrong solution?

(The only test this seems to break is one that expects the zero-width spaces to be present, but I can fix it and add a nested quote test.)

from converse.js.

BetaRays avatar BetaRays commented on June 2, 2024

The zero-width spaces were added in this commit: 97be0bd, but they don’t seem to be the part that fixes the linked issue.
In any case, the zero-width spaces break this line: https://github.com/conversejs/converse.js/blob/master/src/shared/styling.js#L109.

from converse.js.

BetaRays avatar BetaRays commented on June 2, 2024

I also tried this in order to keep the zero-width spaces, but that prevents the removal of > characters, which are added on the next lines.

diff --git a/src/shared/styling.js b/src/shared/styling.js
index bf09a649a..7dba22c45 100644
--- a/src/shared/styling.js
+++ b/src/shared/styling.js
@@ -106,7 +106,7 @@ function getDirectiveLength (d, text, i) {
     const begin = i;
     i += d.length;
     if (isQuoteDirective(d)) {
-        i += text.slice(i).split(/\n[^>]/).shift().length;
+        i += text.slice(i).split(/\n\u200B*[^>\u200B]/).shift().length;
         return i-begin;
     } else if (styling_map[d].type === 'span') {
         const line = text.slice(i).split('\n').shift();

I don’t know if the zero-width spaces have any use besides preventing this removal (which shouldn’t be done in this case).
The only issue I could find with the method removing them is the behavior of

> > a
> >
> > b

(note the absence of a space on the second line)
image

from converse.js.

BetaRays avatar BetaRays commented on June 2, 2024

The issue with a > at the end of a line happens because a newline is considered a space by the \s template.

An option would be to use the explicit form of \s to remove \n: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions/Cheatsheet

from converse.js.

BetaRays avatar BetaRays commented on June 2, 2024

Not using zero-width spaces breaks this message:

> https://conversejs.org/
> https://conversejs.org/

This was actually in a test, I don’t know why I thought tests were succeeding with my solution.

from converse.js.

BetaRays avatar BetaRays commented on June 2, 2024

I found another solution that keeps zero-width spaces in place:

diff --git a/src/shared/styling.js b/src/shared/styling.js
index bf09a649a..efc5fad0a 100644
--- a/src/shared/styling.js
+++ b/src/shared/styling.js
@@ -106,7 +106,7 @@ function getDirectiveLength (d, text, i) {
     const begin = i;
     i += d.length;
     if (isQuoteDirective(d)) {
-        i += text.slice(i).split(/\n[^>]/).shift().length;
+        i += text.slice(i).split(/\n\u200B*[^>\u200B]/).shift().length;
         return i-begin;
     } else if (styling_map[d].type === 'span') {
         const line = text.slice(i).split('\n').shift();
@@ -150,8 +150,7 @@ export function getDirectiveTemplate (d, text, offset, options) {
     if (isQuoteDirective(d)) {
         const newtext = text
             // Don't show the directive itself
-            .replace(/\n>\s/g, '\n\u200B\u200B')
-            .replace(/\n>/g, '\n\u200B')
+            .replace(/\n\u200B*>\s?/g, m => `\n${'\u200B'.repeat(m.length - 1)}`)
             .replace(/\n$/, ''); // Trim line-break at the end
         return template(newtext, offset, options);
     } else {

I’ll make a pull request once I add some tests for nested quotes.

from converse.js.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.