traverse directories for project.toml

[jessfraz]

So @Irev-Dev talked about this but say you have a project dir with subdirectories like kcl-samples, you’d want that to be one project with many sub directories, so instead of setting the units based on just the top level directory we should set the units based on the one closest to the file and then traverse backwards to parents.

this is currently how the cli works and this works really well.

this also means I’d say you clone someone else files into yours in a subdirectory it would work for whatever project.toml they pushed to git

so this is like a huge win.

the other thing is and I can open a separate issue but we should split units setting out of project.toml and because you want to commit your units to git but you don’t wanna commit your theme etc. so we need two files. Settings for stuff that you want in git and user specific shit

[jessfraz]

Here’s the issue for separating the two https://github.com/KittyCAD/modeling-app/issues/3961

[Irev-Dev]

Another thing we talked about somewhat related, putting it here so it doesn't get lost.

When we get to imports and people sharing modules, it makes sense that modules can be unitless (nothing committed for units in the project.toml), so they will default back to the user importing the files base unit.

In this case where you export a unitless bit of configurable geometry (takes input dimensions to update the geometry), but you still need to guarantee the size of some parts of it, for example something that needs to mount on DIN rail, this is when you'll use the mm()/inch() functions. i.e. strategically opting into units for sections of the model.

[jessfraz]

Also just to note i think we should do https://github.com/KittyCAD/modeling-app/issues/3961 BEFORE we implement this issue, because we dont want all settings to change we literally only care about the ones in git, which currently are units :)

[jessfraz]

otherwise your theme could change in sub dirs and that seems like unnessary complexity and who the fuck even wants that, idk supporting that seems like insanity since we already have so many bugs with the theme changing and looking weird, so please dont do that until at least all the other bugs with weird theme thigns are fixed, if you absolutely want to, but i wouldnt do it at all

[lf94]

Aside: I think @franknoirot removed per-project themeing. It's all user-level themeing now.

[jessfraz]

oh thank fucking god

[jessfraz]

less features I LOVE IT

[jessfraz]

there is no sarcasm there, really less features == more well working features

[lf94]

Given I know who you are and how you express yourself I figured :rofl:

[jessfraz]

okay i jjust looked tho and the rust side still has those settings for projects so we should clean that up https://github.com/KittyCAD/modeling-app/blob/main/src/wasm-lib/kcl/src/settings/types/project.rs#L64

[jessfraz]

since we'd be renaming files and migrating shit anyways its a good change to clean up what actually can change app level vs. project level vs. dir level

[Irev-Dev]

Aside: I think @franknoirot removed per-project themeing. It's all user-level themeing now.

I think we should be careful not add anything that could be considered a "preference" to the project.toml in future, units are needed for the model, theme is not haha.

[franknoirot]

Yeah 100% agree @Irev-Dev. I actually don't think we need any project-level preferences (other than possibly themeColor if we think that's worth it), and the only project-level (or dir-level) setting we need is units.

[franknoirot]

But I think we don't need even that if we implement proper unit types in KCL. We do not need this issue if we implement proper units in KCL, and I think it is critical that we do that.

Currently, KCL files cannot fully define concrete geometry on their own. They represent abstract, unit-less geometry until either a unit standard library function (like mm()) is applied or an external unit setting is read in. Put another way:

• circle({ center: [1, 2], radius: 3}, sketch001) means nothing on its own
• circle({ center: [1.in, 2.in], radius: 3.in }, sketch001) is fully descriptive

The fact that our standard library functions take raw number expressions is an implicit external dependency that makes any KCL file not properly stand on its own. This is exemplified by issues like this one, where generated text-to-cad KCL was only correct if it was accompanied by an external unit setting.

If types are a part of the KCL syntax, and therefore the default unit setting was just that, what unit gets written by default when using the UI, then the following becomes true:

• changing the default unit setting never has any impact on or breaks your KCL
• we don't need any project-level settings at all, and therefor don't need to handle any kind of settings hierarchy
• it becomes impossible to receive KCL code that breaks due to mismatched or missing settings
• what is implicit today when you change the "default unit" setting can be made into a safe, explicit command to convert all the units in your file from one unit to another
• we can provide UX to convert an expression of any unit to any other unit, in the code editor and elsewhere

Impact on typing use of KCL is not large

1. We could still support arbitrary math with syntax like (5 ** 2 - someFnThatReturnsANumber(3)).in
2. We could return clear errors when a user tries to type 3 as the radius of a circle, such as:

   Error: this function accepts units of length, not numbers. Please specify a unit eg: `3.in` or `5.mm`

3. Users can still create abstract, unit-less pieces of geometry using functions

There is no impact on point-and-click experience

1. In inputs such as the command bar, a unit dropdown would be added to the end of the KCL expression input, and we would preselect the unit according to their "default unit" preference
2. The lower-right unit control could either be non-desctructive (as @nadr0 has advocated for), and represent the displayed units preference, doing conversion from the units in the KCL code, or work the same as today and change the default unit preference, which would have no destructive impact.

Massive impact on current code

I think this is the biggest issue with implementing this, but I think it's critical we do it now.

[jessfraz]

I dont think it should be required, thats a huge burden in a file where most files are the same unit and we should push people to most files being the same units. I consider this the equivalent of the other unit functions we have which i think i should have made less easy to use because we don't want people uysing them.

Anyways my negatives are:

• most files use the same units everywhere, this is a huge burden for that
• it opens the door for the anti-pattern of mixing units, its a huge context burden to read code with mixed units and actually understand
• we want to make mixing units hard since its a bad pattern, this makes it way too easy

[jessfraz]

i personally loive that the car wheel example breaks when the units are off, BECAUSE WE WANT TO FORCE people to not mix units, because if you dont it scales well and you get so many wins. I don't want to make mixing units easier. It is a terrible pattern and no one will ever use code cad if the file is unreadable.

[jessfraz]

also for libraries we will want "unitless" consider jordans letter library or the zoo logo, those would be unit less and that would work out of the box today!

PLUS for libraries like a car wheel, you could use the unit functions we have today to even make that unitless, anyways I am pro the paradigms we set up for today because they work great with the future paradigms as well

just in case someone calls bullshit on the above, you'd make the car wheel unitless by using the units functions everywhere, which is the burden you have to bear WHEN YOU ARE DOING THE ANTIPATTERN, thats by design

[jessfraz]

anyways I do not want to optimize for the anti-pattern, we need to stop feeling bad about the anti-pattern kinda sucking, BECAUSE IT SHOULD

[franknoirot]

most files use the same units everywhere, this is a huge burden for that

Is the huge burden having .in specified after each raw number? That doesn't seem like a huge burden, maybe I'm missing something.

we want to make mixing units hard since its a bad pattern, this makes it way too easy

Mixed unit parts are incredibly common though; even the category of adapter pipe fittings is massive. In shops that build assemblies, for example, it's really common to have the screw holes in whatever your locality is, but then the important dimensions in whatever your customer's dimensions are. We'd be expecting machine shops with access to metric hardware building for imperial to buy imperial

[jessfraz]

yes but if you mix units, its not that we don't support it today, it just takes more effort, which it should since we don't want people doing it unless super necessary

[jessfraz]

.in is three more characters to type EVERY TIME YOU WRITE SOMETHING thats insane to me

[franknoirot]

I don't think mixed units is an anti-pattern, it's a fact of life. There are many many parts that are built in the world and will that use different unit systems. That must be where I'm misunderstanding.

[franknoirot]

.in is three more characters to type EVERY TIME YOU WRITE SOMETHING thats insane to me

I get that, but it's the same argument for making the circle standard library function be c

[jessfraz]

lets let @JordanNoone and @jgomez720 and @nicboone8 chime in but im super anti doing this even just the context it takes the read the code has an overhead because usually I can look at code and grok it, but like now im like fuck are the units just going to switch somewhere, like i feel like the other is more readable but maybe im just biased

[jessfraz]

c makes no sense and is unreadable, yes you need more characters to understand shit sometimes

[franknoirot]

c makes no sense and is unreadable, yes you need more characters to understand shit sometimes

Yeah it's an extreme example, but I read angledLine({ length: 2, angle: 30 }) and think "2 of WHAT"? And so does our KCL interpreter if you haven't specified a setting in our special file

[jtran]

yes but if you mix units, its not that we don't support it today, it just takes more effort, which it should since we don't want people doing it unless super necessary

I feel like you're not thinking big enough. What I hear you actually saying is that you don't want to degrade the experience when you're only using one kind of unit. I think we can do both.

By adding explicit units, we make the "difficult" case of mixed units easy because the language and tools can help you by providing better error messages (for when you use an angle when a function expects a length) or even automatically converting between inches and millimeters when appropriate.

Have you ever used uom for dealing with measurements in Rust? Compile time checking of units. It's amazing. So hard to go back once you've experienced them.

On the other hand, if everything is unspecified, the tool can't understand what anyone means and can't provide assistance.

[jessfraz]

everything is not unspecified tho, we literally know the units from the units file, and we use that at executor time, i guess you are saying to have it at parse time but im unsure what we gain from that since we do both at the same time in the app / cli anyways

[jessfraz]

I think this all comes down to what the MEs want tbh, if I was an ME i wouldnt want to type three extra characters everytime and have the mental overhead of reading files with units everywhere, but thats just me

[franknoirot]

Yeah totally fair, and I see your point. I don't want to optimize for the typing experience first, and I would weight having units of measure be absolutely explicit in one file over the increase in typing friction. I think it has huge implications for code generation.

[jessfraz]

heres another idea too, because I dont want to discount your assemblies have multiple units, yes they do , maybe you import something from mcmaster or whatever, BUT individual parts do not

so take the wheel thats actually an assembly, but if the lugs etc where seperate parts it would not be mixed units UNTIL the assembly file

you see where im going so THIS better supports the whole traverse directories thing. or per-file units. WHERE per-file YOU really DONT want to be mixing units UNLESS its an assembly. in which case yes mayeb in assemby files we make it easier to mix units BUT do you even need that... if they are all sepoerate parts with their own units being imported jsut being mated together?

[jessfraz]

to even your make the holes in inches to support the inches lugs if you import a variable from a file that is inches it would still be inches, so you COULD do that it would work out of the box with the directory structure and be invisible to the whole units inlined thing, then when you open a file you don't have this awful context switching of units in the file, you are in one unit that entire time, i like not having that mental burden ya know, like oh i open the lug im in inches

[jessfraz]

having import would really make it easier to test all these thoeries 😇 i would like to say lets not close this discussion but let us instead wait until we have import so we can experiment and really see the workflow here, does that work, because i get your points, i do, i just want to make sure adding the mental overhead / typing overhead to every file IS A ABSOLUTE MUST first

[franknoirot]

BUT do you even need that... if they are all sepoerate parts with their own units being imported jsut being mated together?

Oh yeah with my "screws in metric, length in imperial" example, that would probably be covered by what you're talking about for the most part, like the screw files would be all-metric, but the diameter of those holes would be metric too (in this story they have access to metric hardware like screws and drill bits, I'm borrowing from this guy's video, but need a part that has critical dimensions), even if the whole outer dimensions of the assembly are in imperial

[jtran]

if you import a variable from a file that is inches it would still be inches

...regardless of the setting of the user who opened it. I think this point is crucial.

[jessfraz]

right but the diameter of the holes could be imported as a variable from the "imperial file" into a metric file AND IT WOULD WORK thats the coolness of it

[jessfraz]

exactly @jtran

now i really want import to mess around, but yeah maybe we end up realizing thats shit, listen im not always right, but its worth not doing a hiuge overhaul until we know it sucks

[franknoirot]

i like not having that mental burden ya know, like oh i open the lug im in inches

Yeah this is totally fine, and I think having a default unit setting and doing the change outlined in this issue is actually not mutually exclusive with having explicit units always in the file. I spoke too soon earlier, apologies. You could enforce the default unit on a per-directory level with these settings, which would simply dictate what the default selected unit would be when users go to perform actions in that directory, and have the file contain explicit units.

[jessfraz]

dont apologize i like the discussion it made me see a lot of things and it makes us more aware going into import / assemblies on how to make it good and problems that might occur

[franknoirot]

So this traversal effort would still be fruitful. I just don't think it should be done to work around the fact that our geometry-producing standard library functions can take raw numerical values, forcing us to use an external setting to make the geometry concrete.

[jessfraz]

yeah also just to reiterate def push back on work if you see it going nowhere, so im glad you spoke up

[jessfraz]

okay also what would be really cool is if in the assembly files or just anytime you import a variable from somewhere when you hover over it, it told you the units, that would be dope and we could def do something like that in the lsp w the data

[jgomez720]

I realized that my face is blocking the dimensions in SketchUp. But sketching is asking for things like 2'36", 30", 2.32m with every single dimension. I used SketchUp professionally with Spatial for about a year and did not like it. https://github.com/user-attachments/assets/31609634-55e0-41ed-be68-b16c9c76c184

[franknoirot]

@jgomez720 I'm talking about requiring units in the KCL code, not in the command palette input or any other UI. Your settings would merely choose what units in this dropdown I show in the video gets selected by default. The KCL code generated by the extrude command would require a unit value, but your expression input in the UI would not.

https://github.com/user-attachments/assets/ce2b11e8-a0ae-4cd5-82e7-fda118d08123

[jgomez720]

Yeah my point was more that typing in the .in or .mm would get annoying to me after a while. So this

I think this all comes down to what the MEs want tbh, if I was an ME i wouldnt want to type three extra characters everytime and have the mental overhead of reading files with units everywhere, but thats just me

is what I agree with. I'm pretty confident that people are not going to want to add the units to every single dimension. I have a habit of writing my units on constants like so:

// Define constants
const mountingHoleDia = .625 // inches
const baseDia = 4.625 // inches
const pipeDia = 1.25 // inches
const thickness = .625 // inches
const totalThickness = 0.813 // inches
const topTotalDiameter = 2.313 // inches
const bottomThickness = 0.06 // inches
const bottomTotalDiameter = 2.5 // inches
const mountingHolePlacementDiameter = 3.5 // inches
const baseThickness = .625 // inches
const topTotalThickness = totalThickness - (bottomThickness+baseThickness) // inches
const holeLocator = baseDia - 8
const nHoles = 4

and some people would even think that is too much, let alone every number in the file.

The UI stuff you showed is tight btw

[franknoirot]

Totally hear that, but I really think we should have the KCL be the source of truth rather than leave it up to chance to optimize for people who are manually writing all their kcl.

As it stands today, your comments should really say this:

const mountingHoleDia = .625 // inches, hopefully

[franknoirot]

And when something like text-to-kcl starts consuming that code and producing kcl without a settings.toml associated with it, you get the car-wheel problem. Every tool that uses kcl will need to also understand settings.toml

[jgomez720]

The Text-to-KCL point is really good, because giving the user a toml file too would be difficult, especially examples that are not within our database. We are taking a guess of what units it's going to output.

This was something I did wayyyyyyy back in the day, but maybe this crazy idea actually might be worth looking at? Where we have definitions within the KCL file itself. Forgive my psuedocode, I have no idea what to use for these definitions like units or material

// Bracket

!units = "in"
!material = "aluminum-6061"

// Define constants
const sigmaAllow = 35000 // psi
const width = 6
const p = 300 // lbs
const factorOfSafety = 1.2 // unitless
const shelfMountL = 5
const wallMountL = 2

A point that is also worth discussing is non-dimenions units and non-length units. What do we do with sigmaAllow, p, and factorOfSafety in the above example?

[jgomez720]

I still want to pull away from writing the units on every single number. I really don't think people are going to like that. And in other units, are we expecting them to write const sigmaAllow = 35000.psi ?

[franknoirot]

I still want to pull away from writing the units on every single number. I really don't think people are going to like that. And in other units, are we expecting them to write const sigmaAllow = 35000.psi ?

Yes. This becomes even more critical when we support other types of units. Imagine your safety checks being wrong because of some other external file!

Numbers would still be valid in this system, so your factor of safety constant would remain just a number, and could be used in expressions that resolve to a dimension.