feat: 한글의 두음을 반환해주는 acronymizeHangul 함수를 제거합니다.

toss / es-hangul

A modern JavaScript library for handling Hangul characters.

https://es-hangul.slash.page/

MIT License

1.24k stars 83 forks source link

feat: 한글의 두음을 반환해주는 acronymizeHangul 함수를 제거합니다. #180

Closed okinawaa closed 1 month ago

okinawaa commented 1 month ago

Overview

https://github.com/toss/es-hangul/discussions/176 에서 논의한 결과 acronymizeHangul은 한글의 원리를 해결하는것이 아닌, 자바스크립트 일반 메서드로도 해결이 가능하므로 es-hangul에서 제공해주지 않아도 된다고 생각합니다.

s.split(' ').map(s => s[0]).join('')를 하면 사용자 쪽에서도 사실 외부 의존성 없이 해결할 수 있어요. 한글이 아닌 문자는 취급하지 않는다고 써있는 es-hangul 메서드와 다르게 다른 종류의 문자열을 주더라도 잘 동작(내지는 어떻게 동작할지 예측이 가능)하기도 하구요.

PR Checklist

[ ] I read and included theses actions below

I have read the Contributing Guide
I have written documents and tests, if needed.

vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
es-hangul	❌ Failed (Inspect)			Jul 14, 2024 7:52am

changeset-bot[bot] commented 1 month ago

🦋 Changeset detected

Latest commit: 4a4c0d8993cf981907db51f1da6a9f2e404a0549

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

okinawaa commented 1 month ago

@KNU-K 님 안녕하세요. #133 에서 정말 많은 작업해주셨는데요, 이 PR에 대해서 어떻게 생각하시는지 궁금해서 멘션드립니다!

codecov-commenter commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.05%. Comparing base (84eead8) to head (7c8bcab).

Additional details and impacted files

[![Impacted file tree graph](https://app.codecov.io/gh/toss/es-hangul/pull/180/graphs/tree.svg?width=650&height=150&src=pr&token=My9jTW6bSr&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=toss)](https://app.codecov.io/gh/toss/es-hangul/pull/180?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=toss) ```diff @@ Coverage Diff @@ ## v2 #180 +/- ## ========================================== - Coverage 95.08% 95.05% -0.04% ========================================== Files 15 14 -1 Lines 285 283 -2 Branches 67 67 ========================================== - Hits 271 269 -2 Misses 13 13 Partials 1 1 ```

KNU-K commented 1 month ago

제가 느끼기에는 acronymizeHangul이라는 함수는 편의성에 따른 유틸이라고 생각이 됩니다. C++ 로 예를 들어보면 해당 함수 동작에 대한 것을 꽤나 간단하게 만들 수 있지만, 이를 제공하는 것을 볼 수 있습니다. acronymizeHangul도 그와 비슷한 느낌이라고 생각됩니다! 이 부분에서 어떻게 생각하시나요?

KNU-K commented 1 month ago

약간 방향성이 해당 라이브러리에서만 볼 수 있는한글처리 기술에 방향성을 둔 것이라면 지워져도 상관없을 것 같긴 합니다.

okinawaa commented 1 month ago

@KNU-K 각 문자의 앞 글자만 따는 행위는, 한글의 복잡한 초,중,종성 자음 모음 규칙을 알지 못하는 사용자라도 쉽게 구현할 수 있어요. 그래서 es-hangul에서 처리하지 않아도 된다고 생각했습니다!

manudeli commented 1 month ago

@okinawaa conflict 해결부탁드려용