toss / es-hangul

A modern JavaScript library for handling Hangul characters.
https://es-hangul.slash.page/
MIT License
1.34k stars 95 forks source link

feat: 문자열에서 한글만 반환하는 extractHangul을 구현합니다. #130

Closed Collection50 closed 4 months ago

Collection50 commented 4 months ago

Overview

문자열에서 한글만 반환하는 extractHangul을 구현합니다. #108

extractHangul('안녕하세요1234abc'); // '안녕하세요'
extractHangul('abcde'); // ''
extractHangul('안녕하세요ㄱㄴ'); // '안녕하세요ㄱㄴ'
extractHangul('안녕하세요    만나서 반갑습니다'); // '안녕하세요    만나서 반갑습니다'
extractHangul('가나다!-29~라마바.,,사'); // '가나다라마바사'

고민점

PR Checklist

  1. I have read the Contributing Guide
  2. I have written documents and tests, if needed.
changeset-bot[bot] commented 4 months ago

🦋 Changeset detected

Latest commit: f8ef0b9808c53e9953ebfba5e6525cf14b424af5

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

vercel[bot] commented 4 months ago

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

Name Status Preview Comments Updated (UTC)
es-hangul ❌ Failed (Inspect) Jun 29, 2024 2:17pm
codecov-commenter commented 4 months ago

Codecov Report

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

Project coverage is 98.80%. Comparing base (b25fcc5) to head (1d3df38).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/toss/es-hangul/pull/130/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/130?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=toss) ```diff @@ Coverage Diff @@ ## main #130 +/- ## ======================================= Coverage 98.79% 98.80% ======================================= Files 15 16 +1 Lines 249 251 +2 Branches 55 55 ======================================= + Hits 246 248 +2 Misses 3 3 ```
crucifyer commented 4 months ago

이 함수 명칭은 removeUnHangul 같은 지운다는 뜻을 포함했으면 좋겠습니다.

정규식에서 + 는 큰 단위의 반복을 줄여 성능을 높이는 효과가 있습니다. bench: https://jsfiddle.net/crucify/g74b5d1h/

ps. 저 식대로면 ㅠㅠ 같은 문자도 지워지는데 함수의 역할이 무엇인가요?

okinawaa commented 4 months ago

이 함수 명칭은 removeUnHangul 같은 지운다는 뜻을 포함했으면 좋겠습니다.

이 부분에 동의하여, 한글을 지운다는 맥락을 설명할 수 있는 함수명, 혹은 주석에 "한글을 추출" 한다고 작성해주신것을 따라 extractHangul 정도는 어떨까요? 현재 parseHangul이 약간 함수의 동작을 정확히 설명하기에 모호하다고 생각이 들어서요. !

okinawaa commented 4 months ago

escape 문자열도 필터링할 지에 대해 고민 후, 한글과 혼용되어 사용될 것이라 생각하고 필터링하지 않았습니다.

이 부분은 현재 구현대로 띄어쓰기는 삭제하지 않는 것이 좋을 것 같아요~ 라이브러리를 사용하는쪽에서 정말 띄어쓰기를 삭제하고 싶으면 사용하는 쪽에서 제어할 것 이라고 생각해요

Collection50 commented 4 months ago

@crucifyer 피드백 해주셔서 감사합니다 !

Collection50 commented 4 months ago

@okinawaa

이 부분에 동의하여, 한글을 지운다는 맥락을 설명할 수 있는 함수명, 혹은 주석에 "한글을 추출" 한다고 작성해주신것을 따라 extractHangul 정도는 어떨까요? 현재 parseHangul이 약간 함수의 동작을 정확히 설명하기에 모호하다고 생각이 들어서요. !

테스트 코드 보완과 함께 수정 완료 했습니다 !

Collection50 commented 4 months ago

@okinawaa 중복되는 테스트 코드를 삭제하여 가독성 향상했습니다 ! 문서 작성 완료했습니다 !