Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add
windows-terminal
#21913Add
windows-terminal
#21913Changes from 10 commits
ac7d78d
6db1ad4
167351e
541fe75
954e4d7
cc8a1c2
47e913a
24adbf3
2d2a226
c6eaaa6
f8245c0
8612fae
e3735ba
fb4c6da
9e5b695
d9be088
c5b3667
d647d12
6bd6fc0
fabe9ef
2c25497
7c53a32
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Is this creating a
windows-terminal
directory in thePREFIX
root? I think we might want to use a different containing directory, but I am not sure which one is best in Windows.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.
Maybe just
PREFIX/Scripts
?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.
My understanding is that
scripts/
is mostly used for entry-points, so havingwt.cmd
inscripts/
makes sense as it's the entry-point for the Windows Terminal application.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.
I'm not sure it makes sense putting the applications themselves under
scripts/
, but I'm also not sure there's anyconda-forge
convention for where to put applications (particularly onWindows)
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.
If we look at what
nodejs
does, it putsnode-modules
in the top-levelPREFIX
, but it also puts the entry-points inPREFIX
as well:It's likely that the
node.exe
binary expects to find thenode-modules
folder alongside it so that constrains where you could place the executable.IMHO it would probably be cleaner to put the
.cmd
files inscripts/
, but if the actual binaries are inPREFIX
anyway, it probably doesn't have any benefit.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.
I'm fairly sure that
R
used to be installed intoPREFIX/R
but installing the latest version it appears to be installed intoPREFIX/lib/R
...with entry-points installed into
PREFIX/scripts
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.
I'm not sure installing an application under
lib
makes all that much sense, but it also probably doesn't matter - it's transparent to the user callingR.exe
in thePREFIX/Scripts
folder.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.
On Windows, the
dotnet
language installs intoPREFIX/dotnet
but, since I was the one who made that arbitrary decision, it can't really be used as a precedent here 😄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.
At the end of the day, I'm happy to install it wherever you think makes most sense.
Following
R
that would be inPREFIX/lib/windows-terminal
but followingnodejs
that would be inPREFIX/windows-terminal
.I think the status-quo (
nodejs
convention) makes the most sense, but I also don't really mind and am happy to follow any convention (that doesn't require changes to the application)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.
Should this file be named
.cmd
or.bat
? I think there are behaviour differences (see https://stackoverflow.com/questions/148968/windows-batch-files-bat-vs-cmd), but most scripts in conda-forge are .bat. Reading that link though it looks like.cmd
is the modern recommendation? 🤔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.
If we move everything under
Scripts
we might not need this.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.
I view
.bat
and.cmd
as pretty interchangeable. I've never encountered a situation where it mattered though the SO you linked to is pretty informative! It does seem that.cmd
is a bit stricter and generally preferred.My general adhoc convention is to call scripts
.bat
and reserve.cmd
for files which just set up an environment before calling an external binary.Since the script doesn't use any of the mentioned commands where there is a difference in errorlevel handling I think it's a moot point. If you'd prefer it be a
.bat
file I can make that change.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.
If the application was under
scripts/windows-terminal
you'd still need a driver script inscripts/
to call it without having to resort to the full pathname of the executable.