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

(bug: lean4.py): Propagate the lakefile #96

Closed
wants to merge 0 commits into from
Closed

(bug: lean4.py): Propagate the lakefile #96

wants to merge 0 commits into from

Conversation

gaetanserre
Copy link
Contributor

The lakefile path was not propagated to the Lean4 driver. As Lean recently changed their lakefile format from .lean to .toml, it can cause Alectryon + LeanInk to fail on any recent Lean4 project.

@cpitclaudel
Copy link
Owner

Thanks a lot!

Looking at the code, it seems that the --lake argument is removed from the user_args earlier to populate the self.lake_file_path field. Wouldn't it be simpler to leave it in the user_args rather than removing it first and then re-adding it?

@gaetanserre
Copy link
Contributor Author

gaetanserre commented Jul 18, 2024

Indeed, its much better. #fee2fc1 should be fine.

working_directory = os.path.dirname(os.path.realpath(self.lake_file_path))
self.user_args += [self.LAKE_ENV_KEY, self.LAKE_TMP_FILE_PATH]
try:
idx = self.user_args.index("--lake") + 1
Copy link
Owner

Choose a reason for hiding this comment

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

I think this try..catch is fine, but it might be more readable as

lake_idx = self.user_args.index("--lake")
if lake_idx < 0:
  lake_idx = len(self.user_args) # or -2
  self.user_args += …

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess the issue is that if --lake is not in user_args, the .index method will raise an error...

@cpitclaudel
Copy link
Owner

cpitclaudel commented Jul 28, 2024

Sorry, I meant to rebase on main and merge, but Github only let me do the first step and marked the PR closed; can you reopen it by force-pushing to it? (Sorry for the trouble)

I have one last question: this will pass a dummy --lake lakefile.lean if none is specified. Is that desirable / expected? The previous version of the code added this only if a lakefile had been found.

@gaetanserre
Copy link
Contributor Author

Sorry, I meant to rebase on main and merge, but Github only let me do the first step and marked the PR closed; can you reopen it by force-pushing to it? (Sorry for the trouble)

I am sorry, I don't know how to do that right now but I'll look into it.

I have one last question: this will pass a dummy --lake lakefile.lean if none is specified. Is that desirable / expected? The previous version of the code added this only if a lakefile had been found.

I guess it depends on the behaviour of LeanInk. In any case, a Lean project must have a lakefile so if the user is not providing one, I think pass a dummy --lake lakefile.lean is desirable. But we could also throw an error if the user did not provide a lakefile and no "dummy" lakefile (lakefile.{lean|toml}) has been found in the working directory.

@gaetanserre
Copy link
Contributor Author

I made a new PR. See #98.

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.

2 participants