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

Suggest: hljs.listAliases() and hljs.listNames() #2560

Closed
3 of 5 tasks
joshgoebel opened this issue May 16, 2020 · 16 comments
Closed
3 of 5 tasks

Suggest: hljs.listAliases() and hljs.listNames() #2560

joshgoebel opened this issue May 16, 2020 · 16 comments

Comments

@joshgoebel
Copy link
Member

  • hljs.listAliases() → to get the complete list of aliases (with a map to the original language maybe?)
  • hljs.listLanguages()
  • hljs.listNames() → to get the complete list of language names for human to be used in select box, etc.
  • hljs.registerAlias()
  • hljs.registerLanguage()

What do you think?

Originally posted by @taufik-nurrohman in #2558 (comment)

@joshgoebel
Copy link
Member Author

joshgoebel commented May 16, 2020

What good is listNames without the identifiers? I'd think if anything allLanguages or something would be more helpful.

var options = 
  hljs.allLanguages().map(lang => [lang.internalName, lang.prettyName])

But here we quickly hit the fact that our "names" are really nothing more than fancy aliases and languages honestly don't even know their "internal names" LOL.

I'd really rather see individual issues for suggesting new API and it's related discussion though... I don't believe in inventing APIs in the abstract. I'd rather have a concrete real-world use case that is impossible to solve now and someone TRULY needs to solve, and then focus on it. Like the recent addition of registerAliases...

It solved a real problem that was impossible to solve without a new API.

Currently it's VERY easy to build a dropdown with listLanguages paired with getLanguage... I'm not sure we need another function just for simplicities sake.

@joshgoebel joshgoebel changed the title SuggesT: hljs.listAliases() and hljs.listNames() Suggest: hljs.listAliases() and hljs.listNames() May 16, 2020
@joshgoebel
Copy link
Member Author

@taufik-nurrohman Moved your content here.

@joshgoebel
Copy link
Member Author

joshgoebel commented May 16, 2020

listAliases is a better suggestion in that currently there is no external way to access that, but again I'd like to see a real use case. It's possible @woorm might have some thoughts on that.

I tend to err on the side of making new API fight hard to be added rather than just adding anything that "seems nice to have'. Any API we add becomes something we then have to potentially maintain for a long time.

I also wonder if SOME of this might be better solved if we were a real class... and the few people wanting to do complex things with the internals could just subclass us.

@taufik-nurrohman
Copy link
Member

taufik-nurrohman commented May 16, 2020

What good is listNames without the identifiers?

As long as we don’t re-order it, it would be fine.

let languages = hljs.listLanguages();
let names = hljs.listNames();

let results = {};
languages.forEach((language, index) => {
    results[language] = results[names[index]];
});

console.log(results);

Currently it's VERY easy to build a dropdown with listLanguages paired with getLanguage

Then, could we make registerAliases to modify the aliases value from the language definition object too? So that I can drop the listAliases suggestion since it could be built easily from getLanguage too.

let aliases = [];
let names = [];

hljs.listLanguages().forEach(language => {
    let data = hljs.getLanguage(language);
    if (data.aliases) {
        aliases = aliases.concat(data.aliases);
        aliases = arrayUniqueFn(aliases);
    }
    if (data.name) {
        names.push(data.name);
    }
});

function arrayUniqueFn() { ... }

console.log([aliases, names]);

I start thinking about adding hljs.getLanguages() to deprecate hljs.listLanguages(). We can then use Object.keys() to mimic hljs.listLanguages()

let allLanguages = hljs.getLanguages(); // Result will be something like `{css:{},javascript:{}}`

// Deprecate `listLanguages` this way
hljs.listLanguages = () => Object.keys(hljs.getLanguages());

@taufik-nurrohman
Copy link
Member

taufik-nurrohman commented May 16, 2020

Could we make registerAliases to modify aliases values from the language definition object too?

Or simply to merge the registered aliases later to the object data returned by getLanguage (to improve getLanguage instead of registerAliases).

@joshgoebel
Copy link
Member Author

joshgoebel commented May 16, 2020

As long as we don’t re-order it, it would be fine.

Ok forcing people to rely on the ordering of two different methods being the same is not a good API IMHO. :-) It'd probably be better to have one method that just returned all the language data and then anyone could build whatever abstraction on top of that.

Then, could we make registerAliases to modify the aliases value from the language definition object too?

That implies aliases are modifying the core language definition... rather than just being a helpful abstraction that sets above it all - mapping one thing to another. I realize right now you could argue the whole thing is a bit ambiguous. There was indeed no reason (from outside) to think they were different things until we added registerAliases.

I think that was [part] of woorms issue with the API also... he saw registerAlias as an "alter language" operation and I never saw it that way at all - I saw it as a "add mapping" operation, creating a mapping layer that lives outside and above the language data.

And I never really considered this during the discussion of registerAliases. I guess I saw the "internal canonical data" and being different than "aliases registered by a 3rd party" and hence keeping them separate. Hard to say what the right answer here is without knowing how about use cases of aliases. I think right now their major/only?? use case is to allow shorter names to refer to a language such as "lang-jsinstead oflang-javascript`.

So far the only discussion has been a 3rd party wanting to ADD new aliases - and if it does so it could always just keep track of the aliases it added (to have a master list)... so far I'm not sure why a 3rd party needs to retrieve the alias mappings?

This is worth more thought.

let allLanguages = hljs.getLanguages(); // Result will be something like `{css:{},javascript:{}}`

Except I'm not sure I actually LIKE this key/value mapping (and exposing the data this way). It bothers me that what we call "names" are really just fancy "special" aliases. A lot more natural way would be an API that just returned the "cooked" language objects - except then you'd have no way to know their "internal name"...

@joshgoebel
Copy link
Member Author

joshgoebel commented May 16, 2020

I think that was [part] of woorms issue with the API also... he saw registerAlias as an "alter language" operation and I never saw it that way at all - I saw it as a "add mapping" operation, creating a mapping layer that lives outside and above the language data.

So I'm back to why does one 3rd party codebase need to enumerate aliases defined by a second 3rd party codebase? So far the only 'registerAliases' case made at all has been a 3rd party tool WRAPPING us pretty much... so if you wanted to know the full extent of the aliases available I'd suggest it was perhaps the 3rd party library that was adding additional aliases job to provide an API for that.

@joshgoebel
Copy link
Member Author

Of course right now it'd be easy for a 3rd party that INSISTED that data be present in the language.alias themselves to just add it manually themselves - while they were calling registerAliases to update the mapping tables.

@joshgoebel
Copy link
Member Author

joshgoebel commented May 16, 2020

Now I'm realizing an entirely different approach would have been to have made the languages the canonical alias source... then the whole registerAliases function wouldn't even be necessary. :-) But that can't happen without adding some sort of setting/proxy magic to languages (so they could detect themselves being modified). Having to modify them then call resetAliases() or some such would be ultra-yucky.

Amazing how such a simple problem could have so many possible solutions.

@taufik-nurrohman
Copy link
Member

If modifying the core language definition seems too intimidating, then these method pairs might worth to re-think:

  • registerLanguage(name, ...args)
    • getLanguage(name)
    • getLanguages()
  • registerAlias(name, ...args)
    • getAlias(name)
    • getAliases(name)
  • listLanguages()
let aliases = {};
let names = {};

let languages = hljs.getLanguages();

for (let key in languages) {
    let language = languages[key];
    language.name && (names[key] = language.name);
    aliases[key] = hljs.getAliases(key);

    console.log(language.aliases); // List of aliases from the core
    console.log(hljs.getAliases(key)); // List of aliases from the core plus from the aliases table
}

@joshgoebel
Copy link
Member Author

joshgoebel commented May 16, 2020

Do you have some real problem you're trying to solve with your own service/library or you're just suggesting APIs that you think someone might find helpful or just suggesting functionality you think is currently missing?

I don't deny that what you're suggesting is potentially useful - I believe very strongly in YAGNI. So I'm only interested in real life problems, not mere use cases.

Right now I'm more interesting in simplifying/reducing the API than expanding it.

The problem with building APIs to solve problems we don't even have yet is you often build them all wrong (because we're guessing) and then just have to rebuild them later, doing double the work. Or else you make them overly complicated (to avoid rebuilding later), doing triple the work. :-)

It's even possible registerAliases is an example of this because perhaps we built it too quickly - with only a single real-life usage scenario. Preferably we would have had multiple users explaining how they needed such functionality that could have guided the development. Such is what happens when you build functionality with too little data. :-)


If modifying the core language definition seems too intimidating

Not intimidating... I'm just not convinced it's necessary. And I'm not sure "modifying the definition" is the right way of expressing it, since we wouldn't be... the definition to me is the original copy (or raw copy) (which one can always get via rawDefinition()... the runtime copy is a bit different of a thing.

@taufik-nurrohman
Copy link
Member

taufik-nurrohman commented May 16, 2020

Suggesting functionality that I think is currently missing.

Because I already have registerStuff-thing, I felt that I might need the unregisterStuff-thing too in the future.

Right now I'm more interesting in simplifying/reducing the API than expanding it.

Maybe it will be easier to solve this API suggestions by exposing the language data as raw object, that I think removing public methods such as listLanguages and getLanguage will be no problem considering we can re-create those methods ourselves through the raw data that is exposed to public.

(function(hljs) {
    Object.assign(hljs, {
        aliases,
        languages,
        options,
        // getLanguage
        // listLanguages
        // requireLanguage
    });
    window.hljs = hljs;
})({});
// I could make this method by myself
hljs.getLanguage = name => {
    if (typeof hljs.languages[name] === 'function') {
        return hljs.languages[name](hljs);
    }
    return null;
};

@joshgoebel
Copy link
Member Author

joshgoebel commented May 16, 2020

Suggesting functionality that I think is currently missing. Because I already have registerStuff-thing, I felt that I might need the unregisterStuff-thing too in the future.

We might, in the future. Neither of these reasons alone is a good enough reason to add API right now. lowlight has been around at least 4 years and seems to prove that registration is FAR more important than "unregistration".

Closing this for now until we a real need for this to add more context to the discussion.


If you're just looking for something to do to help out there are lots of other tagged open issues you could probably contribute to. :-)

@joshgoebel
Copy link
Member Author

Maybe it will be easier to solve this API suggestions by exposing the language data as raw object

Maybe, it would also make it easier for 3rd party code to mess up our internals badly by accident. Longer-term I think subclassing might be the answer here to "one day i might want to do this with the library"... just subclass it. But that's not a high priority since no one is asking for it and it would require a lot of code work.

@joshgoebel
Copy link
Member Author

joshgoebel commented May 16, 2020

I also tend to apply a little bit of the 37signals philosophy to things like this:

https://basecamp.com/gettingreal/05.7-forget-feature-requests

If these things are critically needed then people will bring real life problems to us that they can't find a way to solve. We don't need to spend time inventing problems that don't exist - when we have real issues that could be fixed that definitely do existing today. :-)

@taufik-nurrohman
Copy link
Member

Yeah. Just treat this suggestion as how you would treat those suggestions on adding line numbers to the highlighted output. This suggestion may not be accepted or will be long overdue, but it is worth discussing. So, when one day there are other people who have similar thoughts, you can direct them to this issue. Thanks for the inputs by the way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants