Skip to content

Fix discovery of project file for exclude #3613

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bishabosha
Copy link
Contributor

@bishabosha bishabosha commented Apr 3, 2025

pick the outermost project file, so that if an excluded directory is itself a scala-cli project then it is ignored.

result of the Scala Tooling Spree April 3 2025

fixes #3546

pick the outermost project file, so that if an excluded directory
is itself a scala-cli project then it is ignored.

fixes VirtusLab#3546
@bishabosha bishabosha force-pushed the fix/exclude-nested-project branch from cf6eee4 to 7b460a8 Compare April 3, 2025 17:15
@bishabosha bishabosha marked this pull request as ready for review April 3, 2025 17:15
@Gedochao Gedochao self-requested a review April 4, 2025 11:09
// this relies upon the inferred workspace root being the outermost directory.
// which is the common case when you pass a single directory.
val projectScalaFileOpt =
projectScalaFileCandidates.sortBy(_.subPath.segments.size).headOption
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while defaulting to the outermost directory doesn't sound like a bad fallback (and we can keep it as fallback), I'm thinking we should favor the project.scala which is closest to the designated workspace for the Scala CLI project - and that may not be the shortest path, actually. Especially since the workspace can be overridden with a command line option.

As in, let's say we have:

── partitionA
   └── outerdir
       └── actualWorkspace
           └── project.scala
── partitionB
    └── outerdir
        └── project.scala

partitionA/outerdir/actualWorkspace/project.scala is longer than partitionB/outerdir/project.scala, but it should still be picked.
That being said your change is strictly better than what we have on main, so I'm okay with that being fixed as a follow-up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so closest to the workspace of Inputs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. Just be wary that it can be overridden with --workspace.

@bishabosha
Copy link
Contributor Author

@Gedochao is there a reason that the paths aren't sorted (e.g. by outermost) before inferring the workspace? ( i guess changing that would be breaking)

@Gedochao
Copy link
Contributor

@Gedochao is there a reason that the paths aren't sorted (e.g. by outermost) before inferring the workspace? ( i guess changing that would be breaking)

No reason that I know of... It should be covered with tests, in case you want to experiment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

//> using exclude does not work when excluded dir has project.scala
2 participants