Skip to content

Comments

Use call-with-current-project when opening unstaged files/directories in legit status#1442

Merged
vindarel merged 2 commits intolem-project:mainfrom
jfaz1:legit-binary-error
Jul 17, 2024
Merged

Use call-with-current-project when opening unstaged files/directories in legit status#1442
vindarel merged 2 commits intolem-project:mainfrom
jfaz1:legit-binary-error

Conversation

@jfaz1
Copy link
Contributor

@jfaz1 jfaz1 commented Jul 15, 2024

Opening files/directories in legit status can sometimes yield an error if you don't have a buffer in the current project open. This changes it so we run call-with-current-project before so we have the right context.

file)))))
(switch-to-buffer buffer)
(editor-error "File ~a doesn't exist." file))))
(uiop:symbol-call :lem/legit :call-with-current-project
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you not use with-current-project inside this command?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

symbol-call doesn't work with macros, and peek-legit is defined before legit so I can't import it directly. Is there another way?

Copy link
Collaborator

@vindarel vindarel Jul 16, 2024

Choose a reason for hiding this comment

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

so, mmh, we can surely move the macro definition in peek-legit or in a common file. I'm annoyed to see call-with-current-project being exported and this lambda.

you ok to give it a try?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I just used symbol-call cause I saw it being used elsewhere in peek-legit, I'll try to find somewhere to move it

Copy link
Collaborator

@vindarel vindarel Jul 16, 2024

Choose a reason for hiding this comment

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

let's say: if this is non trivial, if doing this pulls too many strings, we can do it later. If you find a quick solution, great.

I look at these symbol-call and I wonder if we really need this separation of packages.

Copy link
Contributor Author

@jfaz1 jfaz1 Jul 16, 2024

Choose a reason for hiding this comment

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

Works fine in porcelain, also kinda makes sense for it to be there. Do you prefer if I replace the calls with lem/porcelain:with-current-project or should I just use import-from?
Edit: went ahead and moved it with the explicit call.

Copy link
Member

Choose a reason for hiding this comment

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

The use of symbol-call seems to me to be a sign of poor design.
It is also vulnerable to refactoring, which increases the possibility that changes on the lem side will break the legit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cxxxr ye we're moving away from it, don't worry

Copy link
Collaborator

Choose a reason for hiding this comment

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

great, thank you!

@vindarel vindarel merged commit 3461c5a into lem-project:main Jul 17, 2024
@jfaz1 jfaz1 deleted the legit-binary-error branch July 19, 2024 01:13
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.

3 participants