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

[Request] Auto-apply change language in tools/developer.html #3729

Closed
petelomax opened this issue Mar 3, 2023 · 9 comments
Closed

[Request] Auto-apply change language in tools/developer.html #3729

petelomax opened this issue Mar 3, 2023 · 9 comments
Labels
enhancement An enhancement or new feature good first issue Should be easier for first time contributors help welcome Could use help from community parser

Comments

@petelomax
Copy link
Contributor

petelomax commented Mar 3, 2023

Changing the theme updates automatically but changing the language does not.

Suggested change to tools/developer.html (it made my life much easier anyway):

//    $('.editor button#update-highlighting').click(function(e) {
      function update() {
        var editor = $(this).parents('.editor');
        var language = editor.find('.languages').val();
        var source = editor.find('textarea').val();
        var start_time = +new Date();
// as per https://github.com/highlightjs/highlight.js/issues/2277
//      var result = hljs.getLanguage(language) ? hljs.highlight(language, source, true) : hljs.highlightAuto(source);
        var result = hljs.getLanguage(language) ? hljs.highlight(source, { language, ignoreIllegals: true }) : hljs.highlightAuto(source);
        var rendering_time = +new Date() - start_time;
        vue.code = source;
        vue.language = hljs.getLanguage(language) ? language : '';

        var rendering_stats = result.language + ': relevance ' + (result.relevance );
        if (result.secondBest) {
            rendering_stats += ', ' + result.secondBest.language + ': ' + (result.secondBest.relevance || result.secondBest.r);
        }
        editor.find('.rendering_stats').text(rendering_stats);
        editor.find('.rendering_time').text(rendering_time);
        editor.find('output').text(result.value);
        saveSettings();
//    });
      };
      $('.editor button#update-highlighting').click(update);
      $(".languages").change(update);

Fairly obviously, that's just 3 lines changed and 2 new lines at the end.

Regarding #2277 there is also one in tools/vendor/highlight-vue.min.js, which is also twice as long as it
needs to be, in that it (harmlessly) contains two identical back-to-back copies of the exact same code.

@petelomax petelomax added enhancement An enhancement or new feature parser labels Mar 3, 2023
@joshgoebel
Copy link
Member

The destructiveness of both actions is slightly different but honestly I'm not sure any real thought has even gone into this. If you'd like to make a real PR I'd be happy to test this out and see how it feels and unless that reveals some downside I think this could be a good improvement.

@joshgoebel joshgoebel added help welcome Could use help from community good first issue Should be easier for first time contributors labels Mar 19, 2023
@petelomax
Copy link
Contributor Author

A "real PR"? Seriously? That would save you what, 2 seconds? There's not a chance I could do that in less than ten minutes and I expect it would probably end up being over half an hour.

@joshgoebel
Copy link
Member

probably end up being over half an hour.

Agreed, time is indeed one of the most valuable commodities we have. The hope is always that when someone makes a small contribution that might lead to other small contributions - so any upfront learning gets spread out a bit. To work on developer.html you'd only need learn how to build the project, which is documented.

I suggested a PR for several reasons:

  • This is a good opportunity for an easy contribution.
  • This is not a high priority item, so it's not likely to get done soon without help.
  • Actual PRs have a workflow that makes them much easier to work with than code pasted into an issue.

We're all volunteers here, and core team's time these days is quite limited. I'm slowly grinding on the 11.8 and 12.0 releases. Also: any PR that I author requires review by a second member of the team... where-as it only takes one of us to review incoming PRs.

@shabbir23ah
Copy link
Contributor

I would like to make changes and make a PR. Should I need to update the duplicate line in #2277 as described? Please guide me, I'm trying to make my first contribution.

@joshgoebel
Copy link
Member

Duplicate line? I'm not following, sorry.

@shabbir23ah
Copy link
Contributor

As per my understanding, @petelomax explained that there is a repeated line in tools/vendor/highlight-vue.min.js. (regarding #2277 issue ).

If I'm wrong can you please explain to me, what's the task here?

@joshgoebel
Copy link
Member

joshgoebel commented Apr 29, 2023

Now that the Vue plugin isn't part of the core repo anymore when I find time I'll be reverting the dev tool to plain old JS, no frameworks needed. So I think it's fine to leave as is for now. It won't break until v12, and I'll add removing it to the v12 release list.

Done.

@shabbir23ah
Copy link
Contributor

So now I just need to make changes to tools/developer.html and make PR, right?

@joshgoebel
Copy link
Member

Yep

shabbir23ah added a commit to shabbir23ah/highlight.js that referenced this issue May 1, 2023
I have updated tools/developer.html as described  on issue highlightjs#3729
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature good first issue Should be easier for first time contributors help welcome Could use help from community parser
Projects
None yet
Development

No branches or pull requests

3 participants