-
-
Notifications
You must be signed in to change notification settings - Fork 579
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
Fix Git submodule command on Windows #1764
base: master
Are you sure you want to change the base?
Conversation
a6b7309
to
1604b99
Compare
I'm not particularly concerned about the tests here, but I'm wondering if there's a simpler way to fix this. Also we'll need to be extra clear in the function docstrings as to why this is not a simple string as most other commands. The public docs have to be updated as well to reflect the change. |
projectile.el
Outdated
(_ ""))) | ||
|
||
(defun projectile-get-git-sub-projects-command () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably this should be marked as private and also explain why the command has to be generated at runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this make sense?
(defun projectile--get-git-sub-projects-command ()
"Get the sub-projects command for Git.
Defined as a function in order to generate the properly-quoted command at
runtime."
One thing that's not clear to me is whether this needs to be generated every time or can be generated just the first time (which would mean you can simply move the |
I also thought of this, and the only concern I had is that (if I’ve understood correctly) it would cause problems if the same configuration were reused across platforms. Otherwise I definitely think it would be better to just generate it once. |
I don't quite understand what you mean by this. Can you elaborate a bit? |
My understanding is that the custom variable will be processed once. Say you’re running Emacs on Linux. The custom variable will be set accordingly. If you then reuse that config, including the custom settings, for a Windows machine, the custom variable won’t be updated—it’ll remain in Linux form. Assuming this is so, that might simply be an edge case we can ignore. I don’t know how important it is. |
f5e77a1
to
3721398
Compare
3721398
to
c5e2f9a
Compare
c5e2f9a
to
c902f9f
Compare
c902f9f
to
95c514a
Compare
Projectile produces an error when finding project files in a Git repository with submodules on Windows (#1600). This is because the command to find the submodules is hard-coded, including the quotes. This PR adds a
projectile-get-git-sub-projects-command
function to generate the command with the correct quoting at runtime. It also addsprojectile-git-submodule-command-function
, which is set toprojectile-get-git-sub-projects-command
by default, obsoletingprojectile-git-submodule-command
.I’m not sure I can add tests for this since it’s specific to the combination of Git, submodules, and Windows. I did run
checkdoc
but it showed a whole lot of unrelated errors.Before submitting a PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):
eldev test
)M-x checkdoc
warningsYou've updated the readme (if adding/changing user-visible functionality)Thanks!