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

[Bug]: 괄호 뒤 조사를 처리하지 못하는 문제 #32

Closed natz92 closed 6 months ago

natz92 commented 7 months ago

Bug description

과일(딸기)이 좋다.

위처럼 괄호 뒤 조사의 경우 괄호 앞에 단어를 기준으로 조사를 붙이는게 관행이나, 이를 제대로 인식하지 못합니다.

참고: https://www.korean.go.kr/front/onlineQna/onlineQnaView.do?mn_id=216&qna_seq=252613&pageIndex=1

Expected behavior

No response

To Reproduce

import { josa } from "es-hangul";

const word1 = "비고(총수량)";
const word2 = "총수량(비고)";
const word3 = "()";

console.log(josa(word1, "이/가") + " 비어 있습니다.");
console.log(josa(word2, "이/가") + " 비어 있습니다.");
console.log(josa(word3, "이/가") + " 비어 있습니다.");

const word4 = "대지의 여신(이하 가이아)";
const word5 = "가이아(이하 대지의 여신)";
const word6 = "()";

console.log(josa(word4, "을/를") + " 숭배하라");
console.log(josa(word5, "을/를") + " 숭배하라");
console.log(josa(word6, "을/를") + " 숭배하라");
# 출력값
비고(총수량)가 비어 있습니다.
총수량(비고)가 비어 있습니다.
()가 비어 있습니다.
대지의 여신(이하 가이아)를 숭배하라
가이아(이하 대지의 여신)를 숭배하라
()를 숭배하라

Possible Solution

')' 을 기준으로 조사를 처리하여 발생하는 것으로 보여 괄호 앞 단어를 찾아 그에 맞는 조사를 붙여줘야 할것으로 보입니다.

etc.

No response

okinawaa commented 7 months ago

버그 발견 감사합니다! 충분히 있을법한 유스케이스라고 생각해요.

josa함수 내에서, 아래와 비슷한 인터페이스로 구현해보면 좋을 것 같은데 어떠신가요!? 함수명은 책임에 맡게 적절히 수정이 필요해보이긴 합니다!


export function josa(word: string, josa: JosaOption): string {
  if (word.length === 0) {
    return word;
  }

  const hasBracket = checkHasBracket(word);
  const baseWord = hasBracket ? extractBaseWordByBracket(word) : word

  return word + josaPicker(baseWord, josa);
}
jw-r commented 7 months ago

word가 (), 대지(땅)의 여신이 될 케이스를 고려해 다음과 같은 인터페이스로 구현해봐도 좋겠네요!

export function josa(word: string, josa: JosaOption): string {
  const endsWithBracket = checkEndsWithBracket(word);
  const baseWord = endsWithBracket ? removeLastBracket(word) : word;

  if (baseWord.length === 0) {
    return word;
  }

  return word + josaPicker(baseWord, josa);
}

안녕하세요, @natz92 님

매우 의미있는 버그를 발견해주신 것 같아요! 이 이슈에 대해 해결 계획이 있으신지 궁금합니다. 혹시 아직 구체적인 계획이 없으시다면, 제가 이 이슈를 해결해봐도 괜찮을지 여쭙고 싶습니다!

감사합니다!

natz92 commented 6 months ago

@okinawaa @jw-r 좋은 해결 방안들을 생각해 주셔서 감사합니다! 제가 직접 기여할 계획은 없어서 마음껏 좋은 방법을 적용시켜 주셔도 됩니다.

jw-r commented 6 months ago

@natz92 감사합니다!

제가 이 문제를 해결해보겠습니다.

raon0211 commented 6 months ago

개인적인 의견이지만, 이런 경우는 약간의 엣지케이스에 해당하는데, josa 함수가 여기에 대응하도록 하면 구현이 복잡해지고 유지보수가 힘들어질 수 있을 것 같다는 생각이 있어요.

(유명한 소프트웨어 설계 원칙 가운데 Worse is better 라고 하는 원칙이 있는데, 실용적인 수준으로 유스케이스를 커버하고, 구현의 단순함을 인터페이스의 복잡함보다 우선순위 높게 가져가는 것을 추천합니다.)

josa 함수를 수정하기보다는, 애플리케이션에서 josa.pick() 함수를 이용하여 대응해보는 것에 대해서는 어떻게 보시나요?

import { josa } from 'es-hangul';

const word = `대지의 여신(이하 가이아)`;

const result = `${word}${josa.pick(removeBrackets(word), '이/가')`;

function removeBrackets(str: string) {
  /* 문자열에서 괄호를 제거 */
}
evan-moon commented 6 months ago

저는 https://github.com/toss/es-hangul/pull/34 PR에 코멘트를 남겼는데, 이제보니 이슈가 따로 있었군요!

저도 @raon0211 님과 비슷한 의견인데요.

개인적으로 josa 함수는 명사와의 문법 관계를 다루는 것에만 집중하고, 괄호와 같은 특별한 맥락에 대한 처리는 함수 바깥에서 별도로 처리해주는 것이 이 함수의 관심사에 더 적합하지 않을까...하는 생각이 들었습니다!

const text = '샴푸(보습)';
const josaForText = josa(text.replace(/(.*\)/, ''); // '가';
console.log(`${text}${josaForText}`); // '샴푸(보습)가'

https://github.com/toss/es-hangul/pull/34#pullrequestreview-1999226242

evan-moon commented 6 months ago

해당 이슈는 #34 가 Close되어 의사결정을 마무리하려고 하는데, 혹시 처음 이슈레이징해주신 @natz92 님 의견은 어떠신가요?? 🙏

natz92 commented 6 months ago

@evan-moon 내용 잘 확인했습니다. 종료해주셔도 좋습니다. 다들 제 의견에 참여 해주셔서 감사합니다!