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.

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

Update v.1.3.0 Implementation #1

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

justcontributor
Copy link

기존 기능의 일부 오류들을 수정함과 동시에, 단축키와 스니펫을 추가하고 제한적으로나마 일부 linting을 구현했습니다. 상세한 변경 사항은 CHANGELOG.md 및 기타 파일의 변경 사항을 참고해주시기 바랍니다.

우선 제 개발 환경에서는 확장이 정상적으로 동작함을 확인하였으나, 풀 요청 수락 전 검토(테스트)를 진행해주시기 바랍니다.

@jhk1090
Copy link
Owner

jhk1090 commented May 25, 2024

PR 감사합니다!
코드는 한번 검토해보겠습니다

@jhk1090 jhk1090 added the enhancement New feature or request label May 25, 2024
Copy link
Owner

@jhk1090 jhk1090 left a comment

Choose a reason for hiding this comment

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

제 환경이 윈도우인데 코드는 문제 없이 작동하는 거 같습니다.
단 몇 가지를 수정했고, 검토해야 할 사항이 있습니다.

client/src/extension.ts Show resolved Hide resolved
server/src/server.ts Show resolved Hide resolved
syntaxes/key.tmlanguage.json Show resolved Hide resolved
"name": "variable.parameter.namu",
"patterns": [
{
"include": "source.latex"
Copy link
Owner

Choose a reason for hiding this comment

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

이 부분에서 latex 문법이 컨텐츠에 적용되지 않는데 확인해주실 수 있나요?

Copy link
Author

Choose a reason for hiding this comment

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

기존 언어를 include하여 구현을 성공했으나, 현재 Markdown All in One 확장에 의존적입니다. 의존성을 README.md에 고지하거나, 타 확장에 의존하지 않도록 tmlang 정보를 내장할 필요가 있습니다.

syntaxes/key.tmlanguage.json Outdated Show resolved Hide resolved
() =>
`1단계 문단은 비권장 문법입니다.\n2단계 문단 이상으로 변경할 것을 권장합니다.`
);
createDiagnostics(/^##@(.*)/gm, DiagnosticSeverity.Information, (m) =>
Copy link
Owner

Choose a reason for hiding this comment

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

이 부분은 주석을 풀어버리고 다시 나무마크 문법 하이라이팅으로 처리하는 건 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

고정 주석이 있는 경우 diagnostics 기능으로 밑줄을 긋게 하지 말고, syntax highlighting을 적용하는 선에서 해결하자는 말씀이신가요?

snippets.json Show resolved Hide resolved
Copy link
Owner

@jhk1090 jhk1090 left a comment

Choose a reason for hiding this comment

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

TODO.md에 있는 내용들을 검토해봤습니다.
전반적으로 다 추가하면 좋을 것 같은 기능인거 같습니다!

TODO.md Show resolved Hide resolved
TODO.md Show resolved Hide resolved
TODO.md Outdated Show resolved Hide resolved
@jhk1090 jhk1090 changed the title update to v1.3.0 Update v.1.3.0 Implementation May 25, 2024
@jhk1090 jhk1090 added the good first issue Good for newcomers label May 25, 2024
justcontributor and others added 2 commits May 25, 2024 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants