manual-language-selection #296

having this be case-insensitive is definitely a good call

good catch!

Option<&str> ought to suffice here. Looks like a false-negative of clippy::needless_pass_by_value (which we have on)

Same for resolve_merge_cascading

I feel like "unknown language" is not very descriptive. I'd like this to somehow mention that it has to do with the specified language -- something as simple as "Language specified could not be found".

Also this should arguably be a warn!. I think the "Could not find a supported language ..." thing sets a false precedent -- we should probably be warning there as well. I think we use warnings too sparingly in general, but that can be a discussion for another time -- this is good for now.

Here's the reasoning behind the existing debug!: #135 (comment). I think I forgot about info! at the time, which IMO would've been a better option than debug!. So maybe this could be info! as well.

I think a warning for a manually-specified language makes sense; info for the auto-detection fallback also makes sense.

Could you inline lang_name into the format string please? We have this warning for {e,}println!s, but it doesn't flag the logging functions unfortunately... (since they aren't from std)

Part of me wishes that we'd at least try to LangProfile::detect_from_filetype here... I guess that depends on whether false detection is more probable than failed detection -- I.. don't think it is?

The compelling case, to me, is the .pl extension: it is used for both Perl and Prolog. Any kind of auto-detection there is destined for failure without considering the contents of the file as well (but Perl is certainly the global safer-choice default). My view is that the user said it is language X, let's not second guess if things are going downhill.

I mean even if we were to choose the wrong lang_profile, we'd fail almost immediately after that, when we try to parse the file with the wrong parser. But I see your point -- let's leave this as is

This might be a personal thing, but I find or_else and ok_or_else in the same method chain to be very confusing. Also I think needing to resort to Option::transpose is almost always.. not a good sign.

The following should be identical logic-wise while also being more legible?:

let lang_profile = if let Some(lang_name) = language {
    LangProfile::find_by_name(&lang_name)
        .ok_or_else(|| format!("Unknown language {lang_name}"))?
} else {
    LangProfile::detect_from_filename(fname_base).ok_or_else(|| {
        format!(
            "Could not find a supported language for {}",
            fname_base.display()
        )
    })?
};

It being similar to the thing over at line_merge_and_structured_resolution might be considered an additional benefit

See #296/files (comment)