| Commit message (Collapse) | Author | Age |
|---|
| ... | |
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
A swatch is a live, and ideally editable, example of an element of the service. They serve as:
* Documentation: what is this element, how do you use it, what does it do?
* Demonstration: what does this element look like?
* Manual test scaffolding: when I change this element like _so_, what happens?
Swatches are collectively available under `/.swatch/` on a running instance, and are set up in a separate [group] from the rest of the UI. They do not require setup or login for simplicity's sake and because they don't _do_ anything that requires either of those things.
[group]: https://svelte.dev/docs/kit/advanced-routing#Advanced-layouts-(group)
Swatches are manually curated, for a couple of reasons:
* We lack the technical infrastructure needed to do this based on static analysis; and
* Manual curation lets us include affordances like "recommended values," that would be tricky to express as part of the type or schema for the component.
The tradeoff, however, is that swatches may fall out of step with the components they depic, if not reviewed regularly. I hope that, by making them part of the development process, this risk will be mitigated through regular use.
|
| |\ \
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
The styles for the `MessageInput` and `CreateConversationForm` components assumed that a container div would be present, and the components did not render as intended without that div. Therefore: the divs were "part of" the component in most of the ways that matter, not part of the context in which the component is used.
It turns out that those divs aren't necessary or interesting anyways - they were targets for layout, but the same layout can be achieved without them. This change removes the divs entirely.
Merges component-div-nesting into main.
|
| | | |
| | |
| | |
| | | |
This is an extension of the previous commit: we don't need these divs _at all_ to achieve the layout we want, and we aren't attaching behaviour or semantics to them, so, out they go.
|
| | | |
| | |
| | |
| | |
| | |
| | | |
The styles for the `MessageInput` and `CreateConversationForm` components assume that the container div will be present, and the components will not render as intended without them. Therefore: they are "part of" the component in most of the ways that matter, not part of the context in which the component is used. Moving the divs into the component will make it easier to reuse these components (for example, in swatches).
The diff for this looks worse than it is because of indentation changes.
|
| |\ \ \
| |_|/
|/| |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
hasn't been touched.
Steps to reproduce:
**Note**: You will need to watch the traffic in a DOM inspector; this has no user-observable symptoms because there's presently no error reporting for the login form.
1. In a new private tab, visit the `/login` page of a Pilcrow instance.
2. **Without touching the username or password fields**, click `sign in`.
The client _should_ send a request to `/api/auth/login` with the following payload:
```json
{
"name": "",
"password": ""
}
```
However, it instead sends an empty payload, leading to a 422 Unprocessable Content response as the request is missing required fields. Subsequent requests, or any request after the user enters data in the input fields, are correctly serialized.
Merges login-form-nulls into main.
|
| | | |
| | |
| | |
| | | |
The default state of a `$state()` with no arguments is `undefined`, which was then leaking out of this component if the user clicks `sign in` without changing the values. Axiom, our HTTP client library, suppresses fields with `undefined` values in JSON payloads (sensibly enough), leading to empty requests.
|
| | |/
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Steps to reproduce:
**Note**: You will need to watch the traffic in a DOM inspector; this has no user-observable symptoms because there's presently no error reporting for the login form.
1. In a new private tab, visit the `/login` page of a Pilcrow instance.
2. **Without touching the username or password fields**, click `sign in`.
The client _should_ send a request to `/api/auth/login` with the following payload:
```json
{
"name": "",
"password": ""
}
```
However, it instead sends an empty payload, leading to a 422 Unprocessable Content response as the request is missing required fields. Subsequent requests, or any request after the user enters data in the input fields, are correctly serialized.
|
| |/ |
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The term "channel" for a conversational container has a long and storied history, but is mostly evocative of IRC and of other, ah, "nerd-centric" services. It does show up in more widespread contexts: Discord and Slack both refer to their primary conversational containers as "channels," for example. However, I think it's unnecessary jargon, and I'd like to do away with it.
To that end, this change pervasively changes one term to the other wherever it appears, with the following exceptions:
* A `channel` concept (unrelated to conversations) is also provided by an external library; we can't and shouldn't try to rename that.
* The code to deal with the `pilcrow:channelData` and `pilcrow:lastActiveChannel` local storage properties is still present, to migrate existing data to new keys. It will be removed in a later change.
This is a **breaking API change**. As we are not yet managing any API compatibility promises, this is formally not an issue, but it is something to be aware of practically. The major API changes are:
* Paths beginning with `/api/channels` are now under `/api/conversations`, without other modifications.
* Fields labelled with `channel…` terms are now labelled with `conversation…` terms. For example, a `message` `sent` event is now sent to a `conversation`, not a `channel`.
This is also a **breaking UI change**. Specifically, any saved paths for `/ch/CHANNELID` will now lead to a 404. The corresponding paths are `/c/CONVERSATIONID`. While I've made an effort to migrate the location of stored data, I have not tried to provide adapters to fix this specific issue, because the disruption is short-lived and very easily addressed by opening a channel in the client UI.
This change is obnoxiously large and difficult to review, for which I apologize. If this shows up in `git annotate`, please forgive me. These kinds of renamings are hard to carry out without a major disruption, especially when the concept ("channel" in this case) is used so pervasively throughout the system.
I think it's worth making this change that pervasively so that we don't have an indefinitely-long tail of "well, it's a conversation in the docs, but the table is called `channel` for historical reasons" type issues.
Merges conversations-not-channels into main.
|
| | |
| |
| |
| | |
Existing client state, stored in local storage, is migrated to new keys (that mention "conversation" instead of "channel" where appropriate) the first time the client loads.
|
| | | |
|
| | |
| |
| |
| | |
This is a **breaking change** for essentially all clients. Thankfully, there's presently just the one, so we don't need to go to much effort to accommoate that; the client is modified in this commit to adapt, users can reload their client, and life will go on.
|
| | |
| |
| |
| |
| |
| | |
I've split this from the schema and API changes because, frankly, it's huge. Annoyingly so. There are no semantic changes in this, it's all symbol changes, but there are a _lot_ of them because the term "channel" leaks all over everything in a service whose primary role is managing messages sent to channels (now, conversations).
I found a buggy test while working on this! It's not fixed in this commit, because it felt mean to hide a real change in the middle of this much chaff.
|
| |/
|
|
|
|
| |
I've - somewhat arbitrarily - started renaming column aliases, as well, though the corresponding Rust data model, API fields and nouns, and client code still references them as "channel" (or as derived terms).
As with so many schema changes, this entails a complete rebuild of a substantial portion of the schema. sqlite3 still doesn't have very many `alter table` primitives, for renaming columns in particular.
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
When a channel is deleted, Pilcrow should no longer allow new messages to be sent to it. This is part of the intended lifecycle of a channel, and either clients or the server may rely on it implicitly.
Steps to reproduce:
1. Create a channel:
```bash
% curl \
'http://[::1]:64209/api/channels' \
--header 'cookie: identity=REDACTED' \
--header 'content-type: application/json' \
--data '{"name": "oh no"}'
{"at":"2025-07-01T04:38:47.483235Z","id":"C2s1c6r1sc9921y3","name":"oh no"}
```
Note its ID (here, `C2s1c6r1sc9921y3`).
2. Delete it:
```bash
% curl \
'http://[::1]:64209/api/channels/C2s1c6r1sc9921y3' \
--header 'cookie: identity=REDACTED' \
-XDELETE
{"id":"C2s1c6r1sc9921y3"}
```
Substitute the ID appropriately.
3. Send it a message:
```bash
% curl -v \
'http://[::1]:64209/api/channels/C2s1c6r1sc9921y3' \
--header 'cookie: identity=REDACTED' \
--header 'content-type: application/json' \
--data '{"body": "oh very no"}'
< HTTP/1.1 202 Accepted
< […]
{"at":"2025-07-01T04:39:23.658387Z","channel":"C2s1c6r1sc9921y3","sender":"L3twnpw918cftfkh","id":"Mhx9k6w3wxnk1f4t","body":"oh very no"}
```
Substitute the ID appropriately here, as well.
What **does** happen is, as above, that the message is sent to the channel. It really is, too - you can see it in the ensuing event stream, or in boot. What **should** happen is that the API should reject the third request with an appropriate error - probably a 404. With this change, that's what happens:
```bash
% curl -v \
'http://[::1]:64209/api/channels/C2s1c6r1sc9921y3' \
--header 'cookie: identity=REDACTED' \
--header 'content-type: application/json' \
--data '{"body": "oh very no"}'
< HTTP/1.1 404 Not Found
< […]
channel C2s1c6r1sc9921y3 deleted
```
We had a test case that was intended to catch this. Unfortunately, I got a bit sloppy when writing it; instead of testing "can you send to a channel after deleting it," it was testing "can you send to a randomly-selected channel ID after deleting a channel" - which you can't, but not because of anything to do with deleting channels. The test case is now fixed, and does actually detect this issue.
This defect was not exercised by the client (and in practice probably affected nobody so far), so there are no client changes included. A client that tries this specific operation will get a 404, as above; this may happen if a user tries to send to a channel that is in the process of expiring at that moment, for example.
---
This change also proposes some changes to the conventions used for histories and for database access. It does not carry those changes through, so it introduces some inconsistencies, though I think they're minor.
* For histories: `Channel` histories now have an `as_of` method that takes an instant or a sequence number, and produces a snapshot of the channel at that point in history. It's used here to figure out what the state of the target channel is at the moment we're sending messages to it, but I think this might be more generally useful, as well.
* For database access: inserting a message now takes a `channel::Channel` (snapshot) instead of a `channel::History` as evidence that the caller has actually resolved the ID to an appropriate entity. This works in tandem with the `as_of` snapshot method, and I'm hoping this might help reduce our reliance on passing histories around to database methods if it generalizes well.
Merges send-to-deleted-channel into main.
|
| | |
| |
| |
| | |
I've opted to make it clear in the error message which scenario - deleted vs. non-existant - a channel falls into. This isn't particularly consistent with the rest of the API, so we might need to review this decision later, but it's at least relatively harmless if it's mistaken. (Formally, they're both 404s, so clients that go by error code won't notice.)
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
ID, not to a fictitious ID.
The existing test scenario:
* Create a channel (with ID C1).
* Delete channel C1.
* Roll the dice to invent a channel ID C2.
* Send a message to channel C2.
* Observe that sending fails.
This was not verifying anything about the deleted channel C1 - it was basically reproducing the `nonexistent_channel` test scenario with the most marginal of garnishes on it. This is probably copy-paste damage from when this test was originally written. Sending did fail, so this test scenario passed, but it passed effectively by accident.
The new test scenario:
* Create a channel (with ID C1).
* Delete channel C1.
* Send a message to channel C1.
* Observe that sending fails.
Concerningly, sending does _not_ fail in this scenario (i.e., the test _does_ fail), so the broken test case was masking a real bug.
|
| |/
|
|
|
|
| |
The audience for this is developers looking to make changes to Pilcrow, either on the server, on the included client, or via its data model. Most of the material here is drawn from existing documents, but organized somewhat more coherently.
I've left some space for client documentation, though no such documents exist yet.
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
There are a couple of contributing reasons for this.
* Each client's author - even ourselves - is best positioned to know how best to convert history into state to meet the needs of that specific client. There is (probably) no universal solution.
You can already see this with the built-in client, where unread tracking gets stapled onto snapshots locally and maintained as events roll in, and I would expect this to happen more and more regularly over time. If we ever sprout other clients, I'd also expect their local state to be different.
The API, on the other hand, must expose a surface that's universal to all clients. For boot, that was a very rote list-of-nouns data model. The other option is to expose a surface specific to one client and make other clients accommodate, which is contrary to the goals of this project.
* The need to compute snapshots adds friction when adding or changing the behaviour of the API, even when those changes only tangentially touch `/api/boot`. For example, my work on adding messages to multiple conversations got hung up in trying to figure out how to represent that at boot time, plus how to represent that in the event stream.
* The rationale for sending back a computed snapshot of the state was to avoid having the client replay events from the beginning of time, and to limit the amount of data sent back. This didn't pan out - most snapshots in practice consisted of the same data you'd get from the event stream anyways, almost with a 1:1 correspondence (with a `sent` or `created` event standing in for a `messages`, `channels`, or `users` entry). Exceptions - deleted messages and channels - were rare, and are ephemeral.
* Generating the snapshots requires loading the entire history into memory anyways. We're not saving any server-side IO by computing snapshots, but we are spending server-side compute time to generate them for clients that are then going to throw them away, as above.
This change resolves these tensions by delegating state management _entirely_ to the client, removing the server-side state snapshots. The server communicates in events only.
## Alternatives
I joked to @wlonk that the "2.0-bis" version of this change always returns `resume_point` 0 and an empty events list. That would be correct, and compatible with the client logic in this change, and would actually work. In fact, we could get rid of the event part of `/api/boot` entirely, and require clients to consume the event stream from the beginning every time they reconnect.
The main reason I _don't_ want to do this has to do with reconnects. Right now - both with snapshots, before this change, and with events, after - the client can cleanly delineate "historical" events (to be applied while the state is not presented to the user) and "current" events (to be presented to the user immediately). The `application/event-stream` protocol has no way to make that distinction out of the box, and while we can hack something in, all the approaches I can think of are nasty.
Merges boot-events into main.
|
| | |
| |
| |
| | |
Clients now _must_ construct their state from the event stream; it is no longer possible for them to delegate that work to the server.
|
| | |
| |
| |
| | |
We use the same event processing glue that the client has for keeping up with live events, which means that a significant chunk of state management code goes away entirely.
|
| | |
| |
| |
| | |
The returned events are all events up to and including the `resume_point` in the same response. If combined with the events from `/api/events?resume_point=x`, using the same `resume_point`, the client will have a complete event history, less any events from histories that have been purged.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
These filters are meant to be used with, respectively, `Iterator::filter_map` and `StreamExt::filter_map`. The two operations are conceptually the same - they pass an item from the underlying sequence to a function that returns an option, drops the values for which the function returns `None`, and yields the value inside of `Some` in the resulting sequence.
However, `Iterator::filter_map` takes a function from the iterator elements to `Option<T>`. `StreamExt::filter_map` takes a function from the iterator elements to _a `Future` whose output is `Option<T>`_. As such, you can't easily use functions designed for one use case, for the other. You need an adapter - conventionally, `futures::ready`, if you have a non-async function and need an async one.
This provides two sets of sequence filters:
* `crate::test::fixtures::event` contains functions which return `Option` directly, and which are intended for use with `Iterator::filter_map`.
* `crate::test::fixtures::event::stream` contains lifted versions that return a `Future`, and which are intended for use with `StreamExt::filter_map`.
The lifting is done purely manually. I spent a lot of time writing clever-er versions before deciding on this; those were fun to write, but hell to read and not meaningfully better, and this is test support code, so we want it to be dumb and obvious. Complexity for the sake of intellectual satisfaction is a huge antifeature in this context.
|
| | |
| |
| |
| |
| |
| | |
The _snapshot_ is specifically a snapshot of app state. The purpose of the response struct is to annotate the snapshot with information that isn't from the app, but rather from the request or the web layer. The heartbeat timeout isn't ever used by the app layer in any way; it's used by the Axum handler for `/api/events`, instead.
I straight-up missed this when I wrote the original heartbeat changes.
|
| | |
| |
| |
| | |
These were invalidated by eff129bc1f29bcb1b2b9d10c6b49ab886edc83d6, back in September.
|
| | |
| |
| |
| | |
Missed in 9f7f82dbd9adee8ae18ae7ff2600b3e1dc8fadbc.
|
| |/ |
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
HTTP routes are now defined in a single, unified module, pulling them out of the topical modules they were formerly part of.
This is intended to improve the navigability of the codebase. Previously, finding the handler corresponding to a specific endpoint required prior familiarity, though in practice you could usually guess from topic area. Now, all routes are defined in one place; if you know the path, you can read down the list to find the handler.
Handlers themselves live with the domain they are most appropriately "part of," generally (in this version, universally) in a `handlers` submodule. The handlers themselves have been flattened down; rather than representing a path and a method, they now represent a named operation (which is suspiciously similar to the path in most cases). This means that we no longer have constructs like `crate::ui::routes::ch::channel` - it's now `crate::ui::handlers::channel` instead.
## Disclaimer
I Solemnly Swear I Didn't Change Any Handlers.
## Prior art
I've inadvertently reinvented Django's `urls.py` convention, and I've opted to lean into that.
Merges flatter-routes-reorg into main.
|
| | |
| |
| |
| |
| |
| | |
Each domain module that exposes handlers does so through a `handlers` child module, ideally as a top-level symbol that can be plugged directly into Axum's `MethodRouter`. Modules could make exceptions to this - kill the doctrinaire inside yourself, after all - but none of the API modules that actually exist need such exceptions, and consistency is useful.
The related details of request types, URL types, response types, errors, &c &c are then organized into modules under `handlers`, along with their respective tests.
|
| | |
| |
| |
| |
| |
| |
| |
| | |
HTTP routes are now defined in a single, unified module, pulling them out of the topical modules they were formerly part of.
This is intended to improve the navigability of the codebase. Previously, finding the handler corresponding to a specific endpoint required prior familiarity, though in practice you could usually guess from topic area. Now, all routes are defined in `crate::routes`.
Other than changing visibility, I've avoided making changes to the handlers at the ends of those routes.
|
| |/
|
|
| |
I've replaced it with something more general, which will be applicable no matter how we restructure the routing.
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The two middlewares were identical but for the specific `IntoResponse` impl used to generate the response when setup has not been completed. However, unifying them while still using `from_fn_with_state` lead to this horrorshow:
.route_layer(middleware::from_fn_with_state(
app.clone(),
|state, req, next| {
setup::middeware::setup_required(UNAVAILABLE, state, req, next)
}
))
It's a lot to read, and it surfaces the entire signature of a state-driven middleware `fn` into the call site solely to close over one argument (`UNAVAILABLE`).
Rather than doing that, I've converted this middleware into a full blown Tower middleware, following <https://docs.rs/axum/latest/axum/middleware/index.html#towerservice-and-pinboxdyn-future>. I considered taking this further and implementing a custom future to remove the allocation for `Box::pin`, but honestly, that allocation isn't hurting anyone and this code already got long enough in the translation.
The new API looks like:
.route_layer(setup::Required(app.clone()))
Or like:
.route_layer(setup::Required(app.clone()).with_fallback(RESPONSE))
Part of a broader project to reorganize our routing. This is intended to reduce the number of "why do we have two of these?" questions when the answer isn't very compelling. We had two of these because I needed them to behave slightly differently, and at the time I didn't fully understand how to abstract out that difference. Now I do, thanks to Axum's excellent documentation.
Merges unify-setup-required into main.
|
| | |
| |
| |
| |
| |
| | |
For endpoints that are unavailable, that default behaviour no longer needs to be specified: `Required(app)` will do that for you. For endpoints that are redirects until setup is completed, `Require(app).with_fallback(…response…)` will do that.
To make this a bit harder to break by accident, the default unavailable response is now its own type.
|
| |/
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The two middlewares were identical but for the specific `IntoResponse` impl used to generate the response when setup has not been completed. However, unifying them while still using `from_fn_with_state` lead to this horrorshow:
.route_layer(middleware::from_fn_with_state(
app.clone(),
|state, req, next| {
setup::middeware::setup_required(UNAVAILABLE, state, req, next)
}
))
It's a lot to read, and it surfaces the entire signature of a state-driven middleware `fn` into the call site solely to close over one argument (`UNAVAILABLE`).
Rather than doing that, I've converted this middleware into a full blown Tower middleware, following <https://docs.rs/axum/latest/axum/middleware/index.html#towerservice-and-pinboxdyn-future>. I considered taking this further and implementing a custom future to remove the allocation for `Box::pin`, but honestly, that allocation isn't hurting anyone and this code already got long enough in the translation.
The new API looks like:
.route_layer(setup::Required::or_unavailable(app.clone()))
Or like:
.route_layer(setup::Required::with_fallback(app.clone(), RESPONSE))
One thing I would have liked to have avoided is the additional `app.clone()` argument, but there isn't a way to extract the _state_ from a request inside of an Axum middleware. It has to be passed in externally - that's what `from_fn_with_state` is doing under the hood, as well. Using `State` as an extractor doesn't work; the `State` extractor is special in a _bunch_ of ways, and this is one of them. Other extractors would work. Realistically, I'd probably want to explore interfaces like
.route_layer(setup::Required(app).or_unavailable())
or
.route_layer(app.setup().required().or_unavailable())
|
| |
|
|
|
|
| |
build` fails.
This was missed in 4396c912771f136f7d397a67f247c81532520b85.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
From its documentation:
> This command is similar to `npm install`, except it's meant to be used in automated environments such as test platforms, continuous integration, and deployment -- or any situation where you want to make sure you're doing a clean install of your dependencies.
We don't need a clean install, necessarily - just a complete one that matches the package configuration. However, this command is clearly documented as being used for automated environments, and I think integration with another build tool is close enough to that intention to fit.
It also promises not to rewrite `package.json` or `package-lock.json`. (`npm install`, on the other hand, rewrites `package-lock.json` regularly.) As we do not intend to change the source tree when building it, this is the preferred behaviour.
Finally, this fixes a behaviour I encountered where certain `cargo` commands - sometimes including `cargo build` - could completely reformat `package-lock.json` without any warning and without any user-visible rationale for it.
|
| | |
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| | |
There isn't a corresponding fix script for this as the lockfiles are rebuilt every time you run a command that resolves dependencies, in either `cargo` or `npm`. Furthermore, `cargo build` (and anything else that runs `build.rs`) will implicitly run `npm install` and update `package-lock.json` in the process.
This was originally part of [another proposal][pr-6]. I've broken it out to make the intent clearer, and to make the proposal easier to get a handle on in isolation from other, related changes. Thanks to @wlonk for their input on this!
[pr-6]: https://codeberg.org/ojacobson/pilcrow/pulls/6
Merges prop/lockfile-checks into main.
|
| |/
|
|
| |
There isn't a corresponding fix script for this as the lockfiles are rebuilt every time you run a command that resolves dependencies, in either `cargo` or `npm`. Furthermore, `cargo build` (and anything else that runs `build.rs`) will implicitly run `npm install` and update `package-lock.json` in the process.
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The new `tools/check-lint` script checks lints across _all_ lintable files - JS (through `eslint`), and Rust (through `clippy` and `cargo check`). It also checks `eslint` against the whole project, not just against what's in the `ui` subdir, which means it now catches lintable issues in various JS config files.
This was originally part of [another proposal][pr-6]. I've broken it out to make the intent clearer, and to make the proposal easier to get a handle on in isolation from other, related changes. Thanks to @wlonk for their input on this!
[pr-6]: https://codeberg.org/ojacobson/pilcrow/pulls/6
Merges prop/lint-checks into main.
|
| | |
| |
| |
| | |
This is entirely mechanical; we don't consider coverage reports to be "part of the project," and there's no point in wasting `eslint`'s time (or ours) in reviewing lints there.
|
| |/
|
|
| |
The new `tools/check-lint` script checks lints across _all_ lintable files - JS (through `eslint`), and Rust (through `clippy` and `cargo check`). It also checks `eslint` against the whole project, not just against what's in the `ui` subdir, which means it now catches lintable issues in various JS config files.
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
* Changes JS formatting policy to expect trailing commas in most of the places they're allowed.
* Moves format checking out of `npm` and into tool scripts.
* _Documents_ formatting tools, as well as a shot at a policy for format-only changes designed to alleviate low-value review.
This was originally part of [another proposal][pr-6]. I've broken it out to make the intent clearer, and to make the proposal easier to get a handle on in isolation from other, related changes. Thanks to @wlonk for their input on this!
[pr-6]: https://codeberg.org/ojacobson/pilcrow/pulls/6
Merges prop/project-formatting into main.
|
| | |
| |
| |
| | |
These are ephemeral, and should not even be _considered_ by `prettier`.
|
| | |
| |
| |
| | |
This is purely a stylistic preference. However, it has helped keep diffs smaller (since diffs at the end of an object or array don't require changing the last line of the array), and it's something a lot of tools and IDEs now default to expecting to.
|
| | | |
|
| | |
| |
| |
| |
| |
| | |
The new `tools/check-format` script checks _all_ project formatting - JS (through `prettier`), and Rust (through `rustfmt`). It also checks `prettier` against the whole project, not just against what's in the `ui` subdir, which means it now catches formatting issues in various JS config files (like `.prettierrc` itself).
This commit does not include style _fixes_, which means that it does not pass its own `tools/check-format` script. This is intentional, and is intended to make the Git history a bit easier to reason about; a future commit will include format fixes.
|
| |/
|
|
|
| |
* It no longer mentions files that do not exist in this project.
* It now _does_ mention files that do exist, that `prettier` should never touch, like sqlx's query- metadata JSON files, `package-lock.json`, or our bundled copies of Mermaid.
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The added suppression for `manual_ok_err` is a mixed choice; I'd prefer `r.ok()` in most senses, but `BroadcastStream` is still new enough that I wouldn't be entirely surprised if the Tokio team added new error variants, that we'd want to expressly handle.
I do feel a bit better suppressing individual [`clippy::pedantic`][pedantic] lints; they're allow-by-default for this reason anyways, and I opted into them (see 452c8d0d9edb9894c108b6d577806c7c9d0071dd) knowing that not all of them would be perfectly appropriate.
[pedantic]: https://doc.rust-lang.org/clippy/
Merges prop/missed-lints into main.
|
| | |
| |
| |
| |
| |
| |
| |
| | |
The added suppression for `manual_ok_err` is a mixed choice; I'd prefer `r.ok()` in most senses, but `BroadcastStream` is still new enough that I wouldn't be entirely surprised if the Tokio team added new error variants, that we'd want to expressly handle.
I do feel a bit better suppressing individual [`clippy::pedantic`][pedantic] lints; they're allow-by-default for this reason anyways, and I opted into them (see 452c8d0d9edb9894c108b6d577806c7c9d0071dd) knowing that not all of them would be perfectly appropriate.
[pedantic]: https://doc.rust-lang.org/clippy/
|
