Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should a deeper-nested style specification inherit the colors/style of its parent? #513

Closed
nadmaximus opened this issue Aug 16, 2019 · 13 comments
Labels
feature resolved if issue is resolved, it will be open until merge with master

Comments

@nadmaximus
Copy link

nadmaximus commented Aug 16, 2019

I have question related to jQuery Terminal

This may be a bug, or it may be a bug in my expectations. I would expect that a string like this:

[[;green;black]This text is green but [[u;;]this text is green and underlined] and this is green]

Would produce text that is all green/black, with an underlined section in the middle. However, the underlined text is the default text color.

Is this working as intended?

@jcubic
Copy link
Owner

jcubic commented Aug 16, 2019

Yes, you're right, the middle text should be green, the nested_formatting that is responsible for this is very simple.

This is something that can be fixed.

@jcubic jcubic added the Bug label Aug 16, 2019
@jcubic
Copy link
Owner

jcubic commented Aug 16, 2019

This is the fix:

$.terminal.nested_formatting = function nested_formatting(string) {
    if (!$.terminal.have_formatting(string)) {
        return string;
    }
    var stack = [];
    var re = /((?:\[\[(?:[^\][]|\\\])+\])?(?:[^\][]|\\\])*\]?)/;
    var format_re = /(\[\[(?:[^\][]|\\\])+\])[\s\S]*/;
    var format_split_re = /^\[\[([^;]*);([^;]*);([^;\]]*)/;
    function get_inherit_style(stack) {
        var style = ['','',''];
        if (!stack.length) {
            return style;
        }
        for (var i = stack.length; i--;) {
            var formatting = stack[i].match(format_split_re);
            for (var j = 0; j < style.length; ++j) {
                var value = formatting[j + 1].trim();
                if (value && !style[j]) {
                    style[j] = value;
                }
            }
        }
        return style;
    }
    return string.split(re).filter(Boolean).map(function(string) {
        if (string.match(/^\[\[/)) {
            var formatting = string.replace(format_re, '$1');
            var is_formatting = $.terminal.is_formatting(string);
            string = string.replace(format_split_re, '');
            stack.push(formatting);
            var style = get_inherit_style(stack).join(';');
            if (!is_formatting) {
                string += ']';
            } else {
                stack.pop();
            }
            string = '[[' + style + string;
        } else {
            var pop = false;
            if (string.match(/\]/)) {
                pop = true;
            }
            if (stack.length) {
                var style = '[[' + get_inherit_style(stack).join(';') + ']';
                string = style + string;
            }
            if (pop) {
                stack.pop();
            } else if (stack.length) {
                string += ']';
            }
        }
        return string;
    }).join('');
};
$.terminal.nested_formatting.__meta__ = true;
$.terminal.nested_formatting.__no_warn__ = true;
$.terminal.defaults.formatters = [$.terminal.nested_formatting];

I will also need to write unit tests and I'm not sure but I think this should be behind some kind of option.

See Demo

@nadmaximus
Copy link
Author

Yes I think an option will be useful, specifically to not inherit. If that option were 'x', you could say something like:

[[g;blue;black]Awesome glowing, then [[x;;]unstyled] then more awesome glowing]

Is that what you mean by behind an option?

@nadmaximus
Copy link
Author

In terms of my use case, I am messing with a color/style utility for producing styled strings for jquery.terminal, that works like kleur (or chalk, ansi-colors, etc) does for ansi escape codes.

@jcubic
Copy link
Owner

jcubic commented Aug 16, 2019

By option I'm more like:

$.terminal.nested_formatting.__inherit__ = true;

@nadmaximus
Copy link
Author

nadmaximus commented Aug 16, 2019

Take this line for example: "_The quick_ brown _fox_". How would you style that, since there is no way to remove the italics, if nested formatting inherits?

And if you can't drop specific things like italics, glow, bold, etc....then nested inheritance is not entirely useful. It's almost like you need a - (too bad ! is already used) operator, to say things like [[bu;;]blah blah][[-b;;]blah blah]]

(I'm perfectly understanding that would be a feature rather than a bug fix)

@jcubic
Copy link
Owner

jcubic commented Aug 16, 2019

This is not good example, because you can very easy create this yourself, you don't need nesting in this case:

$.terminal.defaults.formatters.push(
    [/_([^_]*)_/g, '[[u;;]$1]']
);

Demo

@nadmaximus
Copy link
Author

I am enlightened! Thank you.

@jcubic
Copy link
Owner

jcubic commented Aug 18, 2019

Inherit nested formatting will be disabled by default so it don't break existing code. To enable it you need to use:

$.terminal.nested_formatting.__inherit__ = true;

@jcubic jcubic added feature resolved if issue is resolved, it will be open until merge with master and removed Bug labels Aug 18, 2019
@jcubic
Copy link
Owner

jcubic commented Aug 18, 2019

Removing bug label, because it was meant to work this way, there where even tests that confirm that.

jcubic added a commit that referenced this issue Aug 18, 2019
jcubic added a commit that referenced this issue Aug 18, 2019
@jcubic
Copy link
Owner

jcubic commented Aug 18, 2019

You can now use this:

[[b;#fff;]hello [[u-b;;] world] from js]

it will be converted to (by nested formatting):

[[b;#fff;]hello ][[u;#fff;] world][[b;#fff;] from js]

and remove bold from text " world", and " from js" will be bold.

@nadmaximus
Copy link
Author

Excellent, thanks for that

@jcubic
Copy link
Owner

jcubic commented Aug 29, 2019

released in 2.8.0

@jcubic jcubic closed this as completed Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature resolved if issue is resolved, it will be open until merge with master
Projects
None yet
Development

No branches or pull requests

2 participants