Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make --model parameter of host application more flexible #9

Closed
hannobraun opened this issue Nov 22, 2021 · 5 comments
Closed

Make --model parameter of host application more flexible #9

hannobraun opened this issue Nov 22, 2021 · 5 comments
Assignees
Labels
good first issue Good for newcomers type: feature New features and improvements to existing features

Comments

@hannobraun
Copy link
Owner

hannobraun commented Nov 22, 2021

Currently, if you run fj-host -m my-model, it tries to find a model in models/my-model. This is convenient for use in this repository, but it is less convenient for other use cases.

Here's how I could see that being fixed:

  1. If no -m argument is passed and there is a Cargo package in the current directory, load that.
  2. If no -m arugment is passed and there is no Cargo package in the current directory, fall back to current behavior.
  3. If the parameter passed to -m is an existing path, attempt to load a model from there.

Labeling https://github.com/hannobraun/Fornjot/labels/good%20first%20issue, as this is a rather small change that doesn't require detailed knowledge of Fornjot. Some notes on the implementation:

I think it would make sense to add the new logic in model.rs.

@hannobraun hannobraun added type: feature New features and improvements to existing features good first issue Good for newcomers labels Nov 22, 2021
@hannobraun
Copy link
Owner Author

hannobraun commented Nov 22, 2021

edit: Incorporated this comment into the issue description above.


In case anyone wants to look into this:

  • The argument is specified in args.rs.
  • The argument is then used in model.rs.

I think it would make sense to add the logic I described here in model.rs.

@hannobraun hannobraun changed the title Make model parameter of host application more flexible Make --model parameter of host application more flexible Dec 11, 2021
@hannobraun hannobraun added this to the Basic Usability milestone Jan 20, 2022
@hannobraun
Copy link
Owner Author

The issue description specifies "falling back to the current behavior", if all else fails. #123 is relevant here, as it would make said current behavior more sane.

@hannobraun
Copy link
Owner Author

Updated the issue description to make it more clear what can be done to address this issue.

@hannobraun
Copy link
Owner Author

I'm working on this.

@hannobraun hannobraun self-assigned this Mar 16, 2022
@hannobraun
Copy link
Owner Author

I've looked into this, and it has turned out to be surprisingly difficult. The problem is, a model looks like any other Cargo package, so determining whether there is a model in the current directory isn't straight-forward. This could be solved by adding a special file to model directories, or special metadata to the Cargo.toml of models, but that's more complication for the user. I don't think that's worth it right now.

How models look is an area that is going to change a lot in the future, with the planned move to WASM (#71) and support for more modeling languages. Once that has settled down somewhat, I think it's worth revisiting this issue. For now, I've added it to the feature wishlist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers type: feature New features and improvements to existing features
Projects
None yet
Development

No branches or pull requests

1 participant