-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On Wed, May 24, 2023 at 11:53:51AM +0000, Ben Grande wrote: > Contrary to what doc/package-contributions says to do a brief > description, I prefer a long explanation than having to answer questions > in future mails when I could have answered them upfront.
I like this approach :). Thanks for the contribution! > Index: > - Presentation > - Implementation > - Questions > - Packaging > > Presentation > ============ > I am doing vimfiles for the Qrexec Policy, Qrexec Service Policy and > Qrexec Config. Qubes Code Styling was followed when applicable. > You may find the names strange, but let me show what they mean: > 1. qrexecpolicy: /etc/qubes/policy.d/\*.policy > 2. qrexecpolicyservice: /etc/qubes/policy.d/include/* > 3. qrexecconfig: /etc/qubes/rpc-config/* > > 1. Qrexec was chosen instead of Qubes RPC because it is shorter. > 2. Qrexec Policy Service was chosen because it is included by > '!include-service'. The name could also be 'qrexecpolicyinclude'. > 3. Qrexec Config is never mentioned in the documentation by this name, > so I am open to change to a more suitable name, but 'qubesrpcconfig' > was too long for that matter, 2 chars longer makes a huge > difference. > > Licensed under Vim's license, GPL compatible. See ':h license' for more > information on this topic. qubes-core-qrexec is under GPL-2+ according > to debian/copyright and rpm_spec/qubes-qrexec-dom0.spec.in, therefore it > would be okay for Qubes to hold the vimfiles in the same repository, > changing the license if needed for less complications. > > Below I present the features of the plugin. The mail is quite long, but > this is intended to avoid unnecessary back-and-forths. At the end, there > are open questions for the mailing list. > > You can find the code at https://codeberg.org/ben.grande.b/vim-qrexec. > > I saw that Marta Marczykowska-Gorecka is doing the a GUI application > called qubes-policy-editor. I can't deny it has a better usability for > people not used to terminal applications but it doesn't work for > headless dom0, or embedding the policy syntax into fenced code blocks in > Markdown or Rst. Because the syntax also can be used in fenced code > blocks, people who edit the Qrexec Policy documentation will have the > benefit of getting the syntax inline (g:markdown_fenced_languages). In > the end, the user will choose what suits him best, I am just providing > an alternative. Nice! This does mean that the syntax file needs to be secure against maliciously crafted policy. It looks like there are some dynamically generated regular expressions that could be replaced with stridx() and string slicing. Otherwise, the code looks to be pretty good: there are no calls to execute() or system(), and the code doesn’t open any files with paths that are determined by the policy. > Implementation > ============== > Features: > - Syntax Highlighting: strict check, errors are emphasized > - Lint integration: native mode or via plugins (Dispatch, Ale) > - Code completion: based on syntax and previous entries > - Spell file: good words list > > Dependencies > - Vim version 8.2: stable version for Dom0 Fedora 32 and Debian 11 Do later versions work as well? > - qubes-policy-lint: lint tool, optional (needs to be in Dom0) > > The details provided here are intended to be short and explicit of the > project objective, you should definitely read the reference manual at > doc/qrexec.txt to gather more information. > > This plugin focus on readability, lines are broken into multiple lines > if they become more readable and easier to edit, this hugely impacts on > the number of lines, but greatly decreases the difficulty to get a hold > of what the code is doing. Breaking into multiple lines is not common > according to my search to $VIMRUNTIME, but I find the lines that wrap > multiple times difficult to read and edit, so I am not following Vim's > convention, but my own perception of what makes a clean code, despite > legacy. One side effect is that command options might loose syntax > highlighting, but I believe it is fixed on Vim9 and beyond. That is definitely a good idea. Code that is easier to read is easier to review, and code that is easier to review is more likely to be accepted. > Syntax > ------ > The syntax highlighting follows Woju's code[0]. > > The valid highlighting is nice, but the real benefit of using the syntax > file is by making it strict: > - Validation for every field > - No field can exist without a group > - Requires minimum field number to be reached > - Shows clearly where the error is > - Items of the 2nd field and beyond are always contained and only > matched by 'syntax match' option 'nextgroup='. That is awesome! > Lint > ---- > Wrapper around 'makeprg' with qubes-policy-lint for native method. > If you prefer a third-party plugin, variable for Dispatch and lint > script for ALE is defined. > > Lint catches cases where the syntax would not be a good way to show the > problem. There is no perfect solution anyway, it is not possible to > handle runtime bugs. > > Code completion > --------------- > When the field has know possible values such as the default arguments, > tokens, resolutions, parameters, it is added by default to the > completion list. > > For those same fields, the list is incremented for every line that > reaches the minimum number of fields. If a rule has a qube called > 'sourcevm' as the source for the rule, the 'sourcevm' will be added to > the list of possible values for source completion. NICE!!! > Spell checking > -------------- > If you set 'spell', you will benefit by using the plugin spell file as a > secondary good word list, after your primary language. It helps to > reduce the number of false positives spelling errors. Is this handled automatically? > Credits > ------- > Wojtek Porczyk for the initial syntax file[0]. > > References > ---------- > [0] https://github.com/qubesos/qubes-core-qrexec/vim/syntax/qubespolicy.vim > [1] https://github.com/junegunn/vader.vim/issues/221 > > Questions > ========= > I have a few questions, if you would be kind to answer them: > > 1 - Would QubesOS Team package the vimfiles to for Dom0 and DomUs? > Only Dom0 can lint, as it requires the qubes-qrexec-dom0 to be > installed, but DomUs can greatly benefit from this plugin by using > all of the rest it offers, syntax highlighting, code completion, > spell check. Personally I don’t see any reason not to ship them. What would be needed to support linting outside of dom0? > 1.1 - Is VimScript a barrier for inclusion as it greatly decreases the > chances of someone reviewing it? Total lines of code excluding > comments, empty lines or lines that multi-lines: 957, as of > 2023-04-15, first draft of this e-mail. I am not personally proficient in VimScript, but IIUC syntax files are largely declarative, which should help. > 1.2 - How can I help reviewers? I documented the code via comments and > the usage via the help file. Is there anything that I could clarify > further? Don't hesitate to ask questions. > > 2 - There is one binary and only because Vim requires the spell file > to be compiled, it is at spell/qrexec.ascii.add.spl. Information on > how to generate the file is documented at spell/qrexec.ascii.add, > scripting the compilation is possible so you don't depend on me. It > may vary on different Vim versions and must be compiled with the > same version that is gonna later be used by the users else Vim will > throw an error. Simplest option would be to build it in an postinstall script and use a trigger to make sure that it gets updated when the Vim package is updated. > 3 - What is the recommended style for indentation? Tabs? Spaces? > Textwidth? I made some assumptions based on the current files > shipped by Qubes. Tabs are expanded to spaces and indentation is > made as 2 (two) spaces. Textwidth is set at 78 for comments, leaving > 2 columns for the sign column or line number. Rules are wrapped, > they do not break with the textwidth, but comments break after > textwdith for uniformity. Please search the code for > 'g:qrexec_recommended_style' to suggest changes. Qubes OS uses 4 spaces for indentation in C and Python, but I am not sure what the standard is for VimScript. > Note I am not proficient in VimScript, I learned through the examples at > $VIMRUNTIME, so code review is highly appreciated. > > Code is never complete, tasks can be found by searching for "TODO:". > > Packaging > ========= > Currently, there is a Vim syntax file at qubes-core-qrexec repository, > but it is not installed to Dom0. If Qubes Team decides to package the > plugin, I recommend using the path > /usr/share/vim/vimfiles/pack/qubes/start/vim-qrexec, to keep the > plugin files together, organized, instead of dispersed. A package names > "vim-qrexec" would be a meaningful name and easy to search for. > > Furthermore, I didn't put the build files of qubes-skeleton in place > because I am waiting for the acceptance of this proposal, if it is going > to qubes-core-qrexec, if it is going to stay as a separate repository, > if it is going to become a contributed package or denied the inclusion. > > There are no automated tests as I haven't found a suitable framework, > therefore tests are done manually. There is Vader[1], but it not > actively maintained, but it is the easiest one to test syntax. > > -- > Benjamin Grande - -- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEdodNnxM2uiJZBxxxsoi1X/+cIsEFAmRuXgcACgkQsoi1X/+c IsG6dQ/+J/kr4Jayy7zIRUqG4C5Ak0cmRtk/u1OIelNaXjKUWxuzeJ/krfutvsNI 426UIdnn9aZ9vcYWwmUe0M191C8TXty5ihARU0J+1aKRyx6n1GHVVrTkYLDudZY/ dTkW42EGzeU2cAKecttcQl/jY1l/VjT/j81InMf0kF/eSXalCZxTn2U1w0d5560Z 9YETsyTxrxWH6e4ZyRLNcZ22TWgi/0kzMjxAV4YiAaiS5pUwMgr4CFU1GEu1Gdfr k72Xwim7QSg0uTfA22pFtkzwY7EVZ0X9z79Uq1nqiD60v7GsWpNbgqGbvN8ezj/E Varn/wtOVdgxlmeakXNvyfL2yFZ9qKX9ljGcrGhYHQ+uVRLamNaVdc/nNJUOGEcG CQcCc66VfY4xg6173OKv2EZHKT5gQyk31+4S4E0oqzF6P52ZlKPzeoU5k3r14mN2 Ed6OjpwUPgegYyd4ucSpKdGud1lmIPN7PutZXWfx4arfiJsHhEyxAllyLk5Y07pe FFWdBYUqMVI2xggAwMiii6I50wHIo6nKOZ4ckvzGEVylREwaZP/ul5/15lC0KMPZ YjHH3YS/T2Di0Dt04xnqvdzpyvsJsqcjOmSmf1omKFLJ6r5aTRqTaH/GYi9PQMzL LtA17BixHFGvGesXog3HjYtaPdQC+8ZwxhYBTaxD6yF71Fj/9bo= =iOuq -----END PGP SIGNATURE----- -- You received this message because you are subscribed to the Google Groups "qubes-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to qubes-devel+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/qubes-devel/ZG5eCK%2BHm0mYr3mM%40itl-email.