Refactor with parsing related improvements #896
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than wh
10000
at appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
95db01d to
df59537
Compare
3819c00 to
0914bfe
Compare
f46c71f to
27a8b2a
Compare
Merged
73353e3 to
d209fdc
Compare
a646754 to
05aae2f
Compare
f1240be to
a944148
Compare
|
Hmm, the Windows build using Wine sometimes errors with |
72f41c1 to
306a482
Compare
284cb29 to
f091cf7
Compare
`-Wunused-imports`
- KMonad.Prelude:
- GHC.Conc (orElse) is exported by RIO via UnliftIO.STM
- Removed KMonad.Prelude.Definitions as it is empty
- KMonad.Keyboard.IO.Linux.UinputSink:
- UnliftIO.Async (async) is exported by KMonad.Prelude via RIO
- *:
- KMonad.Keyboard.IO is not needed and should not be needed there
`-Wname-shadowing`
Mark with tick except for KMonad.Args.Cmd (cmd) in KMonad.Args
since it doesn't add clarity
`-Wx-partial`
`-Wdodgy-imports`
- KMonad.Prelude:
- No longer a warning, since we dropped support for the GHC version in
question
`cabal repl` should work again with GHC 9.12. see [#10920](https://gitlab.haskell.org/ghc/ghc/-/issues/10920)
Module | Total | Lib | Test
-------------+-------+-----+------
KMonad.Util | 10 | 10 | 0
RIO.Char | 3 | 2 | 1
RIO.Text* | 1 | 1 | 0
-------------+-------+-----+------
| 14 | 13 | 1
* Duplicate import which was qualified and could therefore evade GHC
warnings
Since our `main` function is just a reexport we don't need an extra directory or `base` as a dependency.
Document all language extensions we are using in `kmonad.cabal` (populate `other-extensions`) and enable `DeriveAnyClass` by default.
This commit unifies the following data structures with the help of type families - 'DefSetting' -> 'Parsing' - 'CfgToken' -> 'Token' - 'JCfg' -> 'Joining' - 'Cmd' -> 'Cmd' - 'AppCfg' -> 'App' - 'AppEnv' -> 'Env' Further changes: - Config values are merged via a Monoid instance => 'joinConfig' can use pattern matching for duplication checking - 'output' setting when missing was wrongly reported as 'input' - Disallow compose sequences such as 'ä' as a compose key - Print the reason why a compose key was invalid - You can specify multiple 'defcfg' which get merged - You can now disable 'fallthrough' and 'allow-cmd' via the command line. - Fixed some 'lexeme' stuff - Empty 'defcfg' is a parse error not a join error - 'lexeme (lexeme ...)' - 'DefSetting' equality instance is now a normal instance, since we no longer use it when merging a 'CmdL' and 'PCfg' and only need it for tests.
The extra `foldable1-classes-compat` dependency is as the name suggests a compatibility dependency. We don't need it with GHC >= 9.6. Furthermore `bifunctors` which is used by `lens` already dependens on it, so it's not a new dependency. The only case where it actually is a new dependency is with lts-20.26 (ghc-9.2.8) of stackage, since `bifunctor`s version is older. In this case we also need to add it as an `extra-dependency` since it's also isn't on stackage.
This should not break any existing configs, and improve the error messages somewhat. The messages are still not perfect, as we don't do a separate tokenizing step (which is very hard, since `"` is either the start of a string or a button, depending of the context). This should also reduce the duplicate `lexeme`s sometimes found, and make the code less confusing in regards to when to use them. Furthermore it is more robust, i.e. less order dependence of parsing branches, and less back tracking, as `try` is only applied when we really need it. We let the primitives do `lexeme`, and therefore (mostly) don't need any in other functions. Furthermore we use `delimited` instead of `lexeme`, since this ensures we don't put keywords next to each other (see the downsides below). Technically sometimes a `lexeme` is missing if the token doesn't parse anything. This can happen due to an empty `many` or an `option`. In this case the `lexeme` is should be covered by the previous token. I have tested this against `tutorial.kbd`, my personal config and `kmonad-contrib`. There were only errors in: - nharward's config, since no `defcfg` blocks are defined - `david-janssen/neo-test.kbd`, since this uses unknown compose sequences and has been broken since at least 0.4.0. The downside is, that my favourite config broke*: ``` (defcfgcmp-seqS-+~input(device-file"")cmp-seqP0output(uinput-sink"")) (defsrc-) (deflayer(;; (around-")) ``` * was technically broken before, due to the config joining rewrite fixing duplicate `cmp-seq`s not being an error.
- Unfolding functions
- `pure ()` -> `pass` (reduces parenthesis)
- `lyr` in `joinButton`
- ...
- Replacing `case`s with `maybe` where it makes sense
- Made better use of `Control.Lens` where applicable
- Automatically derived instances where we manually
implemented the default.
- ...
In one instance we had to change `makeClassy` to `makeLenses`,
since this allows non-simple lenses (i.e. Lenses which change the type).
177ad33 to
810dd20
Compare
Requires GHC 9.4
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello,
if anyone wants to review this I suggest, looking through the commits individually (and taking a look at the commit body message if available).
Furthermore I would also collect the questions about why things have been done a certain way till the end,
since sometimes, this is only really useful in combination with later commits.
I don't expect this to get merged (any time soon), it's just my personal effort to clean up the code base somewhat.
This was originally started of while taking a look around some code and I found some areas which could be somewhat improved to aid readability and maintainability.
Then the following things got done (some should probably live in a separate PR, as they fix actual relevant issues).
DeriveAnyClass(also don't add ", commit" suffix, if we don't know it**)
Types submodules into Model, since they fit there more.riot.l.d.r.: No major changes just some minor* ones which bothered me while working on the code base.
If this ever gets reviewed (sorry), I apologize for the large diff and would probably recommend going at it per commit and not in total.
* and some parsing improvements
** when getting the source from the release page or
cabal get