Give a warning when using per-project default units

[nrc]

cc @nadr0

[qa-wolf]

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
modeling-app	✅ Ready (Inspect)	Visit Preview	Mar 6, 2025 2:26am

[codecov]

Codecov Report

Attention: Patch coverage is 98.59155% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.69%. Comparing base (8aa4609) to head (7a54495).

Files with missing lines	Patch %	Lines
rust/kcl-lib/src/execution/exec_ast.rs	93.75%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5628      +/-   ##
==========================================
+ Coverage   85.67%   85.69%   +0.02%     
==========================================
  Files         106      106              
  Lines       41209    41248      +39     
==========================================
+ Hits        35304    35348      +44     
+ Misses       5905     5900       -5     

Flag	Coverage Δ
rust	85.69% <98.59%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nadr0]

I tested this out locally and I was not able to see the warning. I don't know I am doing something wrong?

I ran yarn install && yarn build:wasm. I loaded the KCL file below. I do not see the warning?

sketch002 = startSketchOn('XZ')
sketch001 = startProfileAt([0.0, 0.0], sketch002)
  |> line(end = [100, 0])

[nrc]

Did you set the per project units to something other than mm?

[nadr0]

Did you set the per project units to something other than mm?

Why would that matter? I thought the intention is if you do not have per file units we show you a warning?

It did show the warning after the unit was changed.

I feel that it is a contradiction to say "we want per file units" but don't warn when mm is missing.

[nrc]

Did you set the per project units to something other than mm?

Why would that matter? I thought the intention is if you do not have per file units we show you a warning?

I've not been part of the discussions on the frontend side, but my understanding was that there was a global default (mm, deg) and the settings would only be required if there was a per-project default that overrode that.

I feel that it is a contradiction to say "we want per file units" but don't warn when mm is missing.

My understanding is that we only want per-file units to replace per-project units, not all the time.

Since I haven't seen a discussion of this, I'll just quickly say why I think that's the right approach. Basically it's just boilerplate and it's right at the top of the file, so a new user (or a slightly less new user seeing it for the first time) will see the settings attribute and have to process it (what does this mean? What does this do? Do I need to care about it? Should I look this up now or come back to it later? Is this syntax the same as similar syntax I might see later?) before they see any code which actually does anything. If the benefit is worth the cost, that's fine, but otherwise it is just unnecessary friction (and learning a new programming language is a very friction-y task to start with).

Obviously the counter-argument is that it is better to be explicit, but I'd ask why in this specific case explicit is better. AIUI, the only benefit is that if the user is implicitly using mm and would prefer to use something else, it is not super-obvious how to change, but I'd counter that the UI for changing is pretty visible so it's not an issue. Perhaps there might be some confusion if a file is implicitly mm but imports a file in inches, but given that we should basically do all the conversions for the user (at least that is the plan, though it is not fully implemented yet), I think it's ok.