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

add support for typst #39

Closed
hannesbraune opened this issue Oct 20, 2024 · 37 comments
Closed

add support for typst #39

hannesbraune opened this issue Oct 20, 2024 · 37 comments
Assignees

Comments

@hannesbraune
Copy link

Hi,
thank you for continuing to maintain ltex after issue 884! ltex+ is a real game changer!
It would be great if you could add support for the new typesetting system typst.
Typst is a new markup-based typesetting system that is designed to be as powerful as LaTeX while being much easier to learn and use (https://github.com/typst/typst). (I highly recommend checking it out!)
Thank you very much!

@spitzerd
Copy link
Contributor

Adding Typst is a great idea. Of course, I cannot announce a delivery date. Feel free to submit a pull request to support Typst.
Because there is no Typst parser in Java or Kotlin available, we need to work with regex, similar to reStructuredText

spitzerd added a commit that referenced this issue Nov 3, 2024
@spitzerd
Copy link
Contributor

spitzerd commented Nov 3, 2024

As this topic was requested by two persons (see ltex-plus/vscode-ltex-plus#74), I gave this topic priority.

I added some basic support for Typst with commit 61f20fd. You can start testing and give me some feedback. Be aware that it is not nearly as sophisticated as LaTeX support.

spitzerd added a commit to ltex-plus/vscode-ltex-plus that referenced this issue Nov 3, 2024
@cgahr
Copy link

cgahr commented Nov 7, 2024

Could you please add the following test, to check inline and multiline math equations?

  @Test
  fun testMathModeInline() {
    assertPlainText(
      """
      This is the math mode $A = pi r^2$
      in Typst
      """.trimIndent(),
      "This is the math mode \nin Typst\n",
    )
  }

  @Test
  fun testMathModeMultiline() {
    assertPlainText(
      """
      This is the math mode
      $
        A = pi r^2
      $
      in Typst
      """.trimIndent(),
      "This is the math mode \nin Typst\n",
    )
  }

@cgahr
Copy link

cgahr commented Nov 7, 2024

As far as I can tell, on the current nightly build, it is not working correctly.

@spitzerd
Copy link
Contributor

spitzerd commented Nov 7, 2024

As far as I can tell, on the current nightly build, it is not working correctly.

Can you explain in more detail what is not working? Is it not working at all for you?

@cgahr
Copy link

cgahr commented Nov 7, 2024

As far as I can tell, on the current nightly build, it is not working correctly.

Can you explain in more detail what is not working? Is it not working at all for you?

If I download and run the nightly build from last night, version 18.3.0-alpha.nightly.2024-11-07, I still get diagnostics everywhere.

For example, in the following typst document

= Title

Thiss is a typo.

And an equation
$
  macron(lambda_s)
$

I would expect to get one diagnostic pertaining Thiss. However, I get additional ones in macron(lambda_s), which should not be there.

I cross checked by running ltex-cli-plus and get the same result.

@spitzerd
Copy link
Contributor

spitzerd commented Nov 7, 2024

There was a request to spell check the content of the math mode, see ltex-plus/vscode-ltex-plus#74 (comment)

That's why I activated the spell checking for the math mode again.

I will try to find a solution.

@cgahr
Copy link

cgahr commented Nov 7, 2024

Ahh I see, that makes sense. I think the best way so tackle this would be to not spell check math BUT strings, i.e. text delimited by " in math mode. Does that make sense?

Thanks a lot!

@spitzerd
Copy link
Contributor

spitzerd commented Nov 7, 2024

Your idea is good. I will provide an update in the upcoming days.

@cgahr
Copy link

cgahr commented Nov 9, 2024

I just tested the latest nightly build 09.11.24.

The first file I tested contains the example from the test:

This is the math mode $ A = pi r^2 $
in Typst.
This is also math $ "exercice" 3 + 4$ in Typst.
This is the multi line math mode
$
  A = pi r^2
$
in Typst.

Running ltex-cli-plus --server-command-line ltex-ls-plus test.typ results in the following error:

test2.typ:6:12: info: The currency mark is usually put at the beginning of the number. [CURRENCY]
java.lang.StringIndexOutOfBoundsException: Range [11, 14) out of bounds for length 13
	at java.base/jdk.internal.util.Preconditions$1.apply(Preconditions.java:55)
	at java.base/jdk.internal.util.Preconditions$1.apply(Preconditions.java:52)
	at java.base/jdk.internal.util.Preconditions$4.apply(Preconditions.java:213)
	at java.base/jdk.internal.util.Preconditions$4.apply(Preconditions.java:210)
	at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:98)
	at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckFromToIndex(Preconditions.java:112)
	at java.base/jdk.internal.util.Preconditions.checkFromToIndex(Preconditions.java:349)
	at java.base/java.lang.String.checkBoundsBeginEnd(String.java:4865)
	at java.base/java.lang.String.substring(String.java:2834)
	at org.bsplines.lspcli.client.Checker$Companion.printDiagnostic(Checker.kt:185)
	at org.bsplines.lspcli.client.Checker$Companion.access$printDiagnostic(Checker.kt:135)
	at org.bsplines.lspcli.client.Checker.checkFile(Checker.kt:129)
	at org.bsplines.lspcli.client.Checker.check(Checker.kt:66)
	at org.bsplines.lspcli.client.Checker.check(Checker.kt:39)
	at org.bsplines.lspcli.LspCliLauncher.call(LspCliLauncher.kt:174)
	at org.bsplines.lspcli.LspCliLauncher$Companion.main(LspCliLauncher.kt:212)
	at org.bsplines.lspcli.LspCliLauncher.main(LspCliLauncher.kt)
Nov 09, 2024 7:12:10 PM org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor run
SEVERE: java.io.IOException: Stream closed
org.eclipse.lsp4j.jsonrpc.JsonRpcException: java.io.IOException: Stream closed
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:122)
	at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:113)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
	at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.io.IOException: Stream closed
	at java.base/java.io.BufferedInputStream.getBufIfOpen(BufferedInputStream.java:188)
	at java.base/java.io.BufferedInputStream.getBufIfOpen(BufferedInputStream.java:198)
	at java.base/java.io.BufferedInputStream.implRead(BufferedInputStream.java:329)
	at java.base/java.io.BufferedInputStream.read(BufferedInputStream.java:318)
	at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:79)
	... 7 more

Running the same command with the file

Thiss is a typo.

And an equation
$ macron(lambda_s) $

results in

test.typ:1:1: info: 'Thiss is': Possible spelling mistake found. [MORFOLOGIK_RULE_EN_US]
Thiss is a typo.
Use 'This is'
Use 'Hiss is'
Use 'Thins is'
Use 'Thies is'
Add 'Thiss is' to dictionary
Hide false positive
Disable rule
test.typ:4:10: info: The word 'lambda_s' is not standard English. Did you mean 'lambda’s' (curly apostrophe) or 'lambda's' (straight apostrophe)? [NON_STANDARD_WORD]
$ macron(lambda_s) $
         Use 'lambda’s'
         Use 'lambda's'
         Hide false positive
         Disable rule

Running ltex-ls-plus directly as a lsp in helix does not show any messages at all. In previous version, diagnostics were displayed correctly.

All 3 case do not seem to work correctly, even with different errors

@spitzerd
Copy link
Contributor

spitzerd commented Nov 9, 2024

Maybe your issue is related to the lsp-cli instead of Typst?
See valentjn/lsp-cli#58

Do you get the same error message if you change the file ending to .tex?

spitzerd added a commit to ltex-plus/lsp-cli-plus that referenced this issue Nov 9, 2024
@spitzerd
Copy link
Contributor

spitzerd commented Nov 9, 2024

I added Typst to lsp-cli-plus, see 9db8dec. Because it was not included in lsp-cli-plus, your files were checked as plaintext.

@cgahr
Copy link

cgahr commented Nov 9, 2024

If I change the extension to .tex, I don't get the OutOfBoundsException, so it might be related to referenced bug.

If I run ltex-ls-plus via helix with args --log-file ltex.log, I can confirm that the language server is indeed starting.

The log has the following content (if it helps):

Content-Length: 2656

{"jsonrpc":"2.0","method":"initialize","params":{"capabilities":{"general":{"positionEncodings":["utf-8","utf-32","utf-16"]},"textDocument":{"codeAction":{"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["","quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}},"dataSupport":true,"disabledSupport":true,"isPreferredSupport":true,"resolveSupport":{"properties":["edit","command"]}},"completion":{"completionItem":{"deprecatedSupport":true,"insertReplaceSupport":true,"resolveSupport":{"properties":["documentation","detail","additionalTextEdits"]},"snippetSupport":true,"tagSupport":{"valueSet":[1]}},"completionItemKind":{}},"formatting":{"dynamicRegistration":false},"hover":{"contentFormat":["markdown"]},"inlayHint":{"dynamicRegistration":false},"publishDiagnostics":{"tagSupport":{"valueSet":[1,2]},"versionSupport":true},"rename":{"dynamicRegistration":false,"honorsChangeAnnotations":false,"prepareSupport":true},"signatureHelp":{"signatureInformation":{"activeParameterSupport":true,"documentationFormat":["markdown"],"parameterInformation":{"labelOffsetSupport":true}}}},"window":{"workDoneProgress":true},"workspace":{"applyEdit":true,"configuration":true,"didChangeConfiguration":{"dynamicRegistration":false},"didChangeWatchedFiles":{"dynamicRegistration":true,"relativePatternSupport":false},"executeCommand":{"dynamicRegistration":false},"fileOperations":{"didRename":true,"willRename":true},"inlayHint":{"refreshSupport":false},"symbol":{"dynamicRegistration":false},"workspaceEdit":{"documentChanges":true,"failureHandling":"abort","normalizesLineEndings":false,"resourceOperations":["create","rename","delete"]},"workspaceFolders":true}},"clientInfo":{"name":"helix","version":"24.7"},"initializationOptions":{"ltex":{"additionalRules":{"motherTongue":"de-DE"},"completionEnabled":true,"dictionary":{},"disabledRules":{"en-US":["UPPERCASE_SENTENCE_START"]},"language":"en-US","latex":{"commands":{"\\gls{}":"Dummy","\\item":"dummy"}}}},"processId":18918,"rootPath":"/home/cgahr/coding/thesis","rootUri":"file:///home/cgahr/coding/thesis","workspaceFolders":[{"name":"thesis","uri":"file:///home/cgahr/coding/thesis"}]},"id":0}Nov 09, 2024 11:37:20 PM org.bsplines.ltexls.server.LtexLanguageServer initialize
INFO: ltex-ls 18.3.0-alpha.nightly.2024-11-09 - initializing...
Content-Length: 260

{"jsonrpc":"2.0","id":0,"result":{"capabilities":{"textDocumentSync":1,"completionProvider":{},"codeActionProvider":{"codeActionKinds":["quickfix.ltex.acceptSuggestions"]},"executeCommandProvider":{"commands":["_ltex.checkDocument","_ltex.getServerStatus"]}}}}

Note that I remove the content of my dictionary since it contains some private details.

I added Typst to lsp-cli-plus, see 9db8dec. Because it was not included in lsp-cli-plus, your files were checked as plaintext.

I'll test tomorrow. Thanks a lot!

@cgahr
Copy link

cgahr commented Nov 10, 2024

On the nightly build from 10.11.24, I still get the same error. Is it possible that an incorrect version of ltex-cli-plus is packaged in it?

Also, ltex-ls-plus nerver goes beyond initializing, judging by the logs..

@spitzerd
Copy link
Contributor

Your guess is right, see commit: 2b60af6

I will trigger a nightly release manually so you can start testing again.

@cgahr
Copy link

cgahr commented Nov 10, 2024

Perfect, thanks a lot!

Using the newest nightly build, ltex-cli-plus correctly finds typos and strings in formulas, but ignores formulas otherwise. Thanks a lot!

The only issue that remains is that, at least for me, the lsp does not seem to initialize properly.

For reference, the last 2 lines of the log file. The lsp is starting, but does not finish initializing:

INFO: ltex-ls 18.3.0-alpha.nightly.2024-11-10 - initializing...
Content-Length: 260

{"jsonrpc":"2.0","id":0,"result":{"capabilities":{"textDocumentSync":1,"completionProvider":{},"codeActionProvider":{"codeActionKinds":["quickfix.ltex.acceptSuggestions"]},"executeCommandProvider":{"commands":["_ltex.checkDocument","_ltex.getServerStatus"]}}}}

@cgahr
Copy link

cgahr commented Nov 11, 2024

I have good news! ltex-ls-plus seems to work correctly, I had some problems with my setup, but now the language server starts correctly. Additionally, formulas are not spell checked, but strings inside are!

Some feedback:

Math mode

This snippet

At time $t$ I go home.

causes the following error

It seems like there are too many consecutive spaces here.

which, if I understand your implementation correctly, is the correct error, but caused by the implementation. After all, you completely remove the equation, and afterwards, there are 2 consecutive spaces.

I propose to replace the equation instead, for example with Math, math or Equation, similar to the option to replace latex commands with Dummy.

A similar problem happens for

This is the end at time $t$.

with error message

Don't put a space before the full stop.

Also

Some text is #text("bold", weight: 800).

causes

Don't put a space before the full stop.

Again, replacing the full expression with i.e. Dummy could fix this.

Finally, I'm unsure if spell-checking strings in formulas is worth it: in some cases it definitely helps. In just as many, however, it will just produce false-positives:

For example

$ mat(delim: "(", A, B)$

with Unpaired symbol: ')' seems to be missing.

Maybe it could be a configurable flag?

In any case, I think it would be helpful to replace math equations like

At time $t_"end" "maybe"$ I go home.

with At time Math (end, maybe) I go home.

Labels

I observed that some labels are spellchecked:

@eq:arst

Is the : part of the regex identifying the label?

Thank you very much, it is already much more usable!

@cgahr
Copy link

cgahr commented Nov 11, 2024

Similarly, strings in functions are spellchecked, for example, in the following function:

image("some_text_with_typos.svg")

This potentially creates a significant number of false-positives.

@spitzerd
Copy link
Contributor

Similarly, strings in functions are spellchecked, for example, in the following function

File names in functions can be filtered with a regex. What do you think?

@spitzerd
Copy link
Contributor

Regarding the math mode: There is already a Dummy generator for LaTeX which can we use. It supports also German and French. The space issues can be handled this way.

Is there a documentation about labels in Typst? The labels in the official Typst documentation look different compared to your example.

@cgahr
Copy link

cgahr commented Nov 11, 2024

Similarly, strings in functions are spellchecked, for example, in the following function

File names in functions can be filtered with a regex. What do you think?

I think the easiest thing to do would be the following: if there is a function, everything following the function in () brackets should NOT be spell checked.

I'm unsure how to handle [] brackets following functions. Square brackets contain content. content can be basically everything. Think

#fun(asd: none)[
  Some text
  #anotherfunction()
]

To get started, it probably would be the easiest to just NOT spell check anything in [] brackets. Down the line, I think content in square brackets should be handled as if it were top-level content.

Function (seem to) start with # and can be followed by letters, numbers, -, _ (see https://forum.typst.app/t/what-characters-may-the-names-of-variables-and-functions-consist-of/1133/2)

Regarding the math mode: There is already a Dummy generator for LaTeX which can we use. It supports also German and French. The space issues can be handled this way.

That's what I meant!

Is there a documentation about labels in Typst? The labels in the official Typst documentation look different compared to your example.

see https://typst.app/docs/reference/foundations/label/:

You can create a label by enclosing its name in angle brackets. This works both in markup and code. A label's name can contain letters, numbers, _, -, :, and ..

Basically, <asd_-:.> defines labels, while @asd_-:. references this label.

@spitzerd
Copy link
Contributor

Maybe it could be a configurable flag?

You can use // LTeX: enabled=false and // LTeX: enabled=true to exclude certain lines from being spell checked.

I think the easiest thing to do would be the following: if there is a function, everything following the function in () brackets should NOT be spell checked.

This would cause that documents like this https://typst.app/universe/package/classy-german-invoice wouldn't be spell checked at all, because everything is in brackets. I don't think this is a good approach.

@cgahr
Copy link

cgahr commented Nov 12, 2024

Maybe it could be a configurable flag?

You can use // LTeX: enabled=false and // LTeX: enabled=true to exclude certain lines from being spell checked.

Didn't know that. Thanks! Personally, however, I still think that strings in equations should not be spellchecked.

I think the easiest thing to do would be the following: if there is a function, everything following the function in () brackets should NOT be spell checked.

This would cause that documents like this https://typst.app/universe/package/classy-german-invoice wouldn't be spell checked at all, because everything is in brackets. I don't think this is a good approach.

I see your point. That would indeed be problematic.

From my point of view, the most pressing issues are:

  • everything in equations is being linted
  • label are spell-checked but shouldn't be

In my case, addressing those 2 should fix 90% of all false-positives. When those are done, I can start using ltex "full-time" and report further problems.

Again, thanks a lot for all your work!

@spitzerd
Copy link
Contributor

I am still working on your issues. The label topic is quite simple to solve, but the space issue is hard. But I am sure that there will be a solution.

@cgahr
Copy link

cgahr commented Nov 15, 2024

That's great to hear! If I can help by doing any testing, let me know!

spitzerd added a commit that referenced this issue Nov 15, 2024
…nd handle labels and file names as markup

See #39
@spitzerd
Copy link
Contributor

The issues you mentioned should be fixed and I just triggered a nightly release for you.
Screenshot with spelling mistakes in Typst

@cgahr
Copy link

cgahr commented Nov 18, 2024

I'm currently testing it. It's already working much better, thanks a lot!

I'll get back to you after using it for a bit.

@TheZoq2
Copy link

TheZoq2 commented Nov 18, 2024

Ohh, very cool to see typst support in here!

I noticed a small issue where function calls with content at the end of a line merges the content with the next line. For example

image

As you can see, it thinks there is a textnext word here

@spitzerd
Copy link
Contributor

@TheZoq2 Thank you for your hint. It's fixed now and you can test again using the next nightly release.

@ju6ge
Copy link

ju6ge commented Nov 19, 2024

Hey Guys,

First off you are awesome. This already works pretty well for me. I am testing it with some bigger typst documents.

I would like to humbly add another case which is currently spell checked that might be resolvable.

Consider the following example:

#let params = json("params.json")
#params.xy

As already discussed the String within the json function call is actually spell checked. But this is probably alright because many packages might require text that will be displayed in the Document as function arguments.

More interesting is the case of spell checking the function name itself as well as the part on the next line where the subkey within the json is also spell checked.
grafik

On another note: Regex seems pretty cumbersome to use i a complex setting like typst. And since there are tree sitter grammars availible for typst I would like to point out that using that might be easier than trying to figure out all the edge cases in regex. Especially when more complex functions can be defined within typst. Typst is for better or worse more a programming language than just a markup language …

@cgahr
Copy link

cgahr commented Nov 22, 2024

I also observed that sometimes functions in multiline equations are spellchecked, even if they should not be.

Inlining them solves the issue. Sadly, I have no code reproducing the issue.

spitzerd added a commit that referenced this issue Nov 22, 2024
@spitzerd
Copy link
Contributor

@ju6ge
Your issue is fixed now. I never heard of tree sitter, but it sounds interesting. Redesigning the parsers from regex to tree sitter is most likely a lot of work, but once it's done, more languages can be added quite easily, I think.

@cgahr
If you cannot provide an example, can you provide a PR or a piece of code I should change?

@ju6ge
Copy link

ju6ge commented Nov 23, 2024

Your issue is fixed now. I never heard of tree sitter, but it sounds interesting. Redesigning the parsers from regex to tree sitter is most likely a lot of work, but once it's done, more languages can be added quite easily, I think.

Thanks I will test it as soon as there is a new nightly release.

Regarding Tree Sitter … It is quite a nice tool, though the richness of the Grammatic does depend on the implementation of tree sitter for the specific language. But especially for cases like Spell checking it its quite awesome. Though redesigning LTeX around tree sitter would probably not be any easy task at all. I have no clue if there are any libraries for kotlin availible.

@ju6ge
Copy link

ju6ge commented Nov 23, 2024

If you cannot provide an example, can you provide a PR or a piece of code I should change?

I have seen similar stuff, I probably can dig up an example case from somewhere.

@spitzerd
Copy link
Contributor

Maybe it's a good point in time to create a release and handle the remaining issues (like the math issue you mentioned) in a separate issue and create a 18.3.1 release later on. Unless there is a justified veto, I will create a release in the upcoming days.

@cgahr
Copy link

cgahr commented Nov 27, 2024

I think that's a good idea, I've been daily driving in the last 10 days+, and it's absolutely usable now (it was not before).

@ju6ge
Copy link

ju6ge commented Nov 27, 2024

I think that's a good idea, I've been daily driving in the last 10 days+, and it's absolutely usable now (it was not before).

I agree, all further edge case can be handled in separate issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants