Per-file units (use per-file settings for conversion functions) #5064
Conversation
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
nrc
force-pushed
the
nrc-units-settings
branch
from
f3a7a9b to
94ce0df
Compare
| @@ -198,6 +198,14 @@ impl ExecState { | |||
|
|
|||
| Ok(id) | |||
| } | |||
|
|
|||
| pub fn length_unit(&self) -> UnitLen { | |||
| self.mod_local.settings.default_length_units | |||
There was a problem hiding this comment.
Can we make a new type for these, basically, I think we don't want people to put all the settings that exist today in the file header, it would be a subset (which is units only today) but could grow, of things people would want checked into git. So we seperate out appearance shit, from the shit that matters if that makes sense. For backwards compat we can keep the old struct w units for now, as long as there is a path forward with a new one and we can nuke the field in the old after, if that makes sense
There was a problem hiding this comment.
We still use the old per-project settings as the default if there are no per-file settings (I think the final step in the migration is deciding what to do here, whether we want to keep this behaviour or remove the per-project settings, but that requires a lot of UI changes so I haven't done it here).
There is a separate settings struct for the per-file stuff called MetaSettings which is in the ExecState (c.f., settings in the ExecContext for stuff specified outside the file).
There was a problem hiding this comment.
gotcha gotcha okay yeah i was just mentioning to frank youll likely need some help w the front-end bits here since i think it touches a lot
There was a problem hiding this comment.
MetaSettings is great, sorry i was looking for whatever that was but likely it was in another pr
Signed-off-by: Nick Cameron <[email protected]>
Signed-off-by: Nick Cameron <[email protected]>
nrc
force-pushed
the
nrc-units-settings
branch
from
419f3b0 to
58f55b9
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5064 +/- ##
==========================================
+ Coverage 86.00% 86.08% +0.07%
==========================================
Files 88 88
Lines 31493 31458 -35
==========================================
- Hits 27085 27080 -5
+ Misses 4408 4378 -30
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| } | ||
|
|
||
| #[tokio::test(flavor = "multi_thread")] | ||
| async fn test_units_inner_ft() { |
There was a problem hiding this comment.
We may have lost some tests. I only saw new tests for mm and inch.
There was a problem hiding this comment.
Yeah, I figured the tests were really annoying to reproduce and weren't that important since these functions are going to get removed or changed significantly soon.
…yCAD#5064) * Use the project default units for the per-file unit default values Signed-off-by: Nick Cameron <[email protected]> * Use per-file units in conversion functions Signed-off-by: Nick Cameron <[email protected]> --------- Signed-off-by: Nick Cameron <[email protected]>
cc #3960