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

Fix batch files #698

Merged
merged 1 commit into from
Nov 14, 2015
Merged

Fix batch files #698

merged 1 commit into from
Nov 14, 2015

Conversation

daxgames
Copy link
Member

@daxgames daxgames commented Nov 8, 2015

Replaced %CMDER_ROOT%\ with %CMDER_ROOT% because the variable already ends with \

@@ -1,3 +1,3 @@
@echo off
SET CMDER_ROOT=%~dp0
start %~dp0/vendor/conemu-maximus5/ConEmu.exe /Icon "%CMDER_ROOT%\icons\cmder.ico" /Title Cmder /LoadCfgFile "%CMDER_ROOT%\config\ConEmu.xml"
start %~dp0/vendor/conemu-maximus5/ConEmu.exe /Icon "%CMDER_ROOT%icons\cmder.ico" /Title Cmder /LoadCfgFile "%CMDER_ROOT%config\ConEmu.xml"
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like an oversight: the normal launcher does not use /LoadCfgFile anymore

@daxgames
Copy link
Member Author

daxgames commented Nov 8, 2015

I am also working on enabling the use of the new git bash. Should be ready soon

@daxgames
Copy link
Member Author

daxgames commented Nov 8, 2015

It will be a separate pull request though

@Stanzilla
Copy link
Member

I don't even know if we want to keep the bat around, it's only used for Windows XP anyway, no?

@daxgames
Copy link
Member Author

daxgames commented Nov 8, 2015

It is in the source so i fixed it. Do with it whatever you feel is necessary.

@Stanzilla
Copy link
Member

I think it looks weird without the \in the paths :/

@daxgames
Copy link
Member Author

It might look wierd but it is correct.

It looks even wierder when you look at the %path% or a variable that has
'' in it.

On November 12, 2015 6:54:47 AM Benjamin Staneck notifications@github.com
wrote:

I think it looks weird without the \in the paths :/


Reply to this email directly or view it on GitHub:
#698 (comment)

@Stanzilla
Copy link
Member

Well actually it might be the better way to just remove the \ from the variable, don't you think?

@daxgames
Copy link
Member Author

We could do that to get rid of the wierdness.

That would also require changes to my other pull request as it behaves in a manner consistent with what init.bat does and expects %CMDER_ROOT% and $CMDER_ROOT both end with either a '' or '/'.

I can work on both to correct.

@Stanzilla
Copy link
Member

I would prefer dropping the slash from the variable.

@cmderdev/trusted-contributors @cmderdev/owners any objections to changing the root variable to drop the slash like that?

Also @daxgames can we leave daxgames@e56e006 out of this PR, I don
t really think we need your personal files in the .gitignore, I know it does not hurt either but yeah.

@daxgames daxgames force-pushed the fix_batch_files branch 5 times, most recently from ebef0f7 to 184344c Compare November 12, 2015 18:39
@daxgames
Copy link
Member Author

I actually modified the .gitignore in my last commit so it is relevant to all and does not contain any personal files. I only added the config/user-* files that are auto created by launching shells in this build.

If you still feel it should be left out then leave it out.

@Stanzilla
Copy link
Member

Nope, that's good. Can you squash please?

@daxgames
Copy link
Member Author

Squashed

@daxgames
Copy link
Member Author

hang on something got messed up in the squash let me fix it.

@daxgames
Copy link
Member Author

Squash fixed. One commit 3 files changed. Sorry about that somehow I combined two branches. in the previous squash

@Stanzilla
Copy link
Member

Great, thank you!

Stanzilla added a commit that referenced this pull request Nov 14, 2015
@Stanzilla Stanzilla merged commit 10d0431 into cmderdev:development Nov 14, 2015
@daxgames daxgames deleted the fix_batch_files branch February 29, 2016 13:45
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.

4 participants