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

customizable file type mapping #3273

Closed
wants to merge 5 commits into from
Closed

customizable file type mapping #3273

wants to merge 5 commits into from

Conversation

sbromberger
Copy link
Contributor

@sbromberger sbromberger commented Jul 31, 2022

ref: #3164

This PR introduces a new [editor.file-types] config section that will allow custom mappings between the default file type strings and user-defined custom strings. An example:

[editor.file-types]
"rust" = "rs"

will remap the "rust" file type string to "rs" (currently used by the statusline). Glyphs/icons should be supported, if available in the user's font definition.

cc @Etienne-K

@AlexanderBrevig
Copy link
Contributor

Meybe this should be under the statusline key? As I understand it, this only affects the rendered output in the statusline. Right?

@sbromberger
Copy link
Contributor Author

sbromberger commented Jul 31, 2022

As I understand it, this only affects the rendered output in the statusline. Right?

Right now that's the case, but it doesn't have to be (see disc).

@the-mikedavis
Copy link
Member

This key name is confusing: looking just at the toml I would think this makes helix think any .rs files are rust (i.e. the file-types key in languages.toml)

@sbromberger
Copy link
Contributor Author

sbromberger commented Jul 31, 2022

The key name is derived from the default strings ("rust", "typescript", "javascript", etc.) returned from context.doc.language_id(). Is there a better way of doing this map without having to redefine a bunch of language-specific identifiers?

(or are you talking about the editor.file-type key? That's easy to change. Do you have a suggested alternative?

Some possibilities:
editor.file-type-indicator, editor.file-type-description)

@the-mikedavis
Copy link
Member

I mean the editor.file-type key, editor.file-type-indicator seems like a good choice

@sbromberger
Copy link
Contributor Author

sbromberger commented Jul 31, 2022

I kept it plural (file-type-indicators) since you might have more than one defined in this section.

Note that the statusline refers to file-type when it uses this string. I tried making this obvious in the docs.

@sbromberger
Copy link
Contributor Author

sbromberger commented Jul 31, 2022

Just in case anyone wants to try this with nerdfonts, here's a quick file I put together that has all the filetypes I and Helix know about: https://gist.github.com/sbromberger/20ba8b7174ecacad43a10a402446d6fa

These probably won't show up in your browser but if you have nerdfonts and download the file, you should see the glyphs. You should be able to append this file to your config.toml and be good to go.

@AlexanderBrevig
Copy link
Contributor

Here's a version that falls back to use the scope identifier if no LSP is provided.
Since Rust has an LSP then rust will stay as rust. Since markdown has no LSP then it will use md as that is its scope ident.

fn render_file_type<F>(context: &mut RenderContext, write: F)
where
    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
{
    let lsp_language_id = context.doc.language_id().unwrap_or("text");
    let mut lsp_language_fallback = String::from("lsp.");
    lsp_language_fallback.push_str(lsp_language_id);
    let (_, file_ext) = context
        .doc
        .language()
        .unwrap_or(lsp_language_fallback.as_str())
        .rsplit_once('.')
        .expect("Language scope should have ident.ext format");
    let editor_config = context.editor.config();
    let file_type = editor_config
        .file_type_indicators
        .get(file_ext)
        .map(|x| x.as_str())
        .unwrap_or(lsp_language_id);

    write(context, format!(" {} ", file_type), None);
}

Not overly testet but what I did test seemed to work.
May serve as a point of discussion, I don't expect the code to be used.

Nice feature BTW 👍

@sbromberger
Copy link
Contributor Author

I think this is ready for review. The bug in language identification is unrelated to this PR but fixing it will definitely make this feature more accurate.

@@ -330,6 +330,12 @@ where
F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
{
let file_type = context.doc.language_id().unwrap_or("text");
Copy link
Member

Choose a reason for hiding this comment

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

Somewhat unrelated to this change, I think this might be better off as the scope field from languages.toml which is context.doc.language().unwrap_or("text"). That way Rust is "source.rust", toml is "source.toml", html is "text.html.basic", etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you suggest any changes to this PR? My opinion is that we should have a function whose sole purpose is to return the language. Right now there are several possible ways to do it, each with their own issues.

Copy link
Member

Choose a reason for hiding this comment

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

we should have a function whose sole purpose is to return the language

The trouble is: what is the language? We could say it's the scope or we could say it's the name (language_id but not language_id(), see also #3338). It depends on what we want the public API of language names to be: is the LanguageConfiguration.language_id an implementation detail or is it ok for configurations to use them?

I'd be hesitant to merge this PR until we figure that out since this PR introduces a way for configurations to depend on language names/ids/scopes.

@archseer what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR merely provides a mapping from the current language reporting functionality so that we can use custom strings that don't take up 1/4 of the status line. I'd be really disappointed if this PR was held up as a result of a feature that is still unimplemented.

Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
@sbromberger sbromberger closed this by deleting the head repository Sep 3, 2022
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

Successfully merging this pull request may close these issues.

4 participants