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.

Sign up for GitHub

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

Always map enry->bblfsh language names #251

Merged
merged 2 commits into from
Feb 14, 2019
Merged

Always map enry->bblfsh language names #251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b2a5ae6
daemon: always map enry->bblfsh language names
bzz Feb 13, 2019
07ce206
Update daemon/language.go
creachadair Feb 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion daemon/language.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,14 @@ func GetLanguage(filename string, content []byte) string {
return lang
}

lang = strings.ToLower(lang)
return normalize(lang)
}

// normalize maps enry language names to the bblfsh ones.
// TODO(bzz): remove this as soon as language aliases are supported in bblfsh
// driver manifest.
func normalize(languageName string) string {
lang := strings.ToLower(languageName)
lang = strings.Replace(lang, " ", "-", -1)
lang = strings.Replace(lang, "+", "p", -1)
lang = strings.Replace(lang, "#", "sharp", -1)
Copy link
Member

Choose a reason for hiding this comment

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

This is an interesting way of solving this :)

Do you think making it an explicit map makes sense?

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 is an interesting way of solving this :)

Well, it's the smallest possible change consistent with current behavior :)

As this is a workaround, I would prefer to keep this change as least invasive as possible.

Given that we do not have much tests for it and that the current approach already works (TM) for API calls \wo the language - I would postponed changing implementation until proper alias support.

Expand Down
4 changes: 4 additions & 0 deletions daemon/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ func (s *ServiceV2) selectPool(rctx context.Context, language, content, filename
return "", nil, err
}
language = lang
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this one need a normalize call too?

Copy link
Contributor Author

@bzz bzz Feb 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This one will already have it normalized after s.detectLanguage that calls normalize underneath

} else { // always re-map enry->bblfsh language names
language = normalize(language)
}

dp, err := s.daemon.DriverPool(ctx, language)
Expand Down Expand Up @@ -225,6 +227,8 @@ func (s *Service) selectPool(ctx context.Context, language, content, filename st
return language, nil, ErrLanguageDetection.New()
}
logrus.Debugf("detected language %q, filename %q", language, filename)
} else { // always re-map enry->bblfsh language names
language = normalize(language)
}

dp, err := s.daemon.DriverPool(ctx, language)
Expand Down