Skip to content

Compiler vlty: support LanguageTool as installed by pacman#1821

Closed
matze-dd wants to merge 5 commits intolervag:masterfrom
matze-dd:master
Closed

Compiler vlty: support LanguageTool as installed by pacman#1821
matze-dd wants to merge 5 commits intolervag:masterfrom
matze-dd:master

Conversation

@matze-dd
Copy link
Copy Markdown
Contributor

@matze-dd matze-dd commented Oct 7, 2020

This PR addresses an issue raised in the discussion of vlty. Under Arch Linux, one now can use LanguageTool as installed with

pacman -S languagetool

The minimal vimrc is given in the documentation.
This requires YaLafi version 1.1.5. It has been uploaded to PyPI, such that it is available via pip.

let &l:makeprg =
\ s:python . ' -m yalafi.shell'
\ . ' --lt-directory ' . s:vlty.lt_directory
\ . ' --lt-command "' . s:vlty.lt_command . '"'
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Does it make sense to always supply the --lt-command option, or should this only be supplied if lt_command is available?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is just the version with the least key strokes. I will change that.


lt_directory~
Path to the `LanguageTool` software.
lt_command~
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I like it better if there's an empty line between the descriptions. Also, I'm curious if lt_command perhaps should be the first entry here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I like it better if there's an empty line between the descriptions.

OK, I will insert a blank line.

Also, I'm curious if lt_command perhaps should be the first entry here?

Don't know, which key is more important. But I assume that most people will install LanguageTool "manually".

doc/vimtex.txt Outdated
instance the file `languagetool-server.jar`. A separated `:compiler vlty` will
raise an error message, if some component cannot be found.
instance the file `languagetool-server.jar`.
If `LanguageTool` has been installed via a packet manager as given above,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I prefer the paragraphs to be full "blocks" of text, so move If ... back to the previous line. If you meant to start a new paragraph, then add an empty line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK.

finish
endif
else
if !filereadable(fnamemodify(s:vlty.lt_directory
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think it is better to use elseif here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My first version was this way, and it is in fact synonymous here. But then, I thought the current code would be more readable. I will change that back.

@lervag
Copy link
Copy Markdown
Owner

lervag commented Oct 7, 2020

This PR addresses an issue raised in the discussion of vlty. Under Arch Linux, one now can use LanguageTool as installed with

pacman -S languagetool

The minimal vimrc is given in the documentation.
This requires YaLafi version 1.1.5. It has been uploaded to PyPI, such that it is available via pip.

Great, thanks! I appreciate the update, I only had a few minor comments!

@matze-dd
Copy link
Copy Markdown
Contributor Author

matze-dd commented Oct 7, 2020

Great, thanks! I appreciate the update, I only had a few minor comments!

My thanks go back for your great project :)

I will incorporate the changes and make a new comment afterwards.

@matze-dd
Copy link
Copy Markdown
Contributor Author

matze-dd commented Oct 7, 2020

The changes are included. Only for the order of lt_directory and lt_command, things are kept. I think, most people will use the manual LanguageTool installation, although this is only a guess.

@lervag
Copy link
Copy Markdown
Owner

lervag commented Oct 7, 2020

Thanks! I merging this now.

lervag added a commit that referenced this pull request Oct 7, 2020
@lervag lervag closed this Oct 7, 2020
@lervag
Copy link
Copy Markdown
Owner

lervag commented Oct 7, 2020

The changes are included. Only for the order of lt_directory and lt_command, things are kept. I think, most people will use the manual LanguageTool installation, although this is only a guess.

Seems sensible. Again, thanks! :)

@matze-dd
Copy link
Copy Markdown
Contributor Author

matze-dd commented Oct 8, 2020

Thank you for merging and the final adjustments! These are all improvements.

@lervag
Copy link
Copy Markdown
Owner

lervag commented Oct 8, 2020

My pleasure :)

lervag added a commit that referenced this pull request Oct 14, 2020
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

Successfully merging this pull request may close these issues.

2 participants