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

Added code to check for the existence of a customized ini file.. #427

Merged
merged 3 commits into from
Dec 4, 2015

Conversation

kodybrown
Copy link
Contributor

No description provided.

@Jackbennett Jackbennett added the C++ label Oct 7, 2015
@MartiUK
Copy link
Member

MartiUK commented Nov 19, 2015

Could you write some documentation about your change, for new users?

@kodybrown
Copy link
Contributor Author

Looks for settings in the following file first config\\ConEmu-%COMPUTERNAME%.xml. If that file is not found, then it will look in the default location config\\ConEmu.xml.

@kodybrown
Copy link
Contributor Author

I updated the README in my fork, is that what you're talking about? I can add some code-comments also?

@kodybrown
Copy link
Contributor Author

I also added a couple comments in the changed code. Can I update this pull request to use my latest version or do I need to create a new pull request?

@Stanzilla
Copy link
Member

You can squash and force push to your branch to update the PR

@MartiUK
Copy link
Member

MartiUK commented Nov 23, 2015

Just commit and push to your fork, it'll update the PR.

@daxgames
Copy link
Member

Should this look for ConEmu-%computername%.xml or user-ConEmu.xml? The latter fits more with what is going on with user config files for the various supported shells.

I would also suggest an addition to the .gitignore of config/ConEmu-*.xml if we stick with the %computername% route. user-ConEmu.xml would already be ignored based on the current development .gitignore.

@kodybrown
Copy link
Contributor Author

Do you mean %USERNAME%-ConEmu.xml? Using the username wouldn't help me, since I have the same user name on all of my machines. But, I would be fine using %ComputerName%-ConEmu.xml... ?

@daxgames
Copy link
Member

No. The current development branch supports the following user shell specific config files:

Powershell = config/user-profile.ps1
Cmder = config/user-startup.cmd
Bash = config/user-cmder.sh

I suggested 'user-conemu.xml' to fit the pattern established by the above files and because currently the development '.gitignore' ignores 'config/user-*'.

I guess I did not understand what you were trying to accomplish, it is clearer now. It sounds like you are looking for a different conemu config based on the name of the system? Not sure why but everyone has different needs.

I guess if you wanted to fit the established pattern AND have what you need it could be 'config/user-%computername%-ConEmu.xml'.

I like configurability/flexibility so I like it what you are doing, I just saw an existing pattern and this did not quite fit the pattern so I thought I'd ask.

EDIT: I personally like the ability to feed the exe an xml file on the command line that way I can create shortcuts to launch whatever I want.

@kodybrown
Copy link
Contributor Author

I see what you mean now!

I like being able to pass the settings/config/xml file to the exe as well, but I've found that most things I need are taken care of by using the ComputerName envar. (I have made similar changes to my own versions of other apps too, such as Notepad2.)

I'm totally fine with config/user-%ComputerName%-ConEmu.xml. Should I change it before finishing up this PR?

@MartiUK
Copy link
Member

MartiUK commented Nov 24, 2015

Is there a need to include the computer name env variable in the filename? I guess the standard now is to have these user- files available.

@MartiUK MartiUK added this to the 1.3 milestone Nov 24, 2015
@kodybrown
Copy link
Contributor Author

I think it is still important to have the computer name in the settings file. Although, I think it makes more sense with the computer name at the end.. as in: config/user-ConEmu-%ComputerName%.xml. Then, if that file doesn't exist it will look for config/user-ConEmu.xml..

@MartiUK
Copy link
Member

MartiUK commented Nov 24, 2015

Sounds good to me.

This was referenced Nov 24, 2015
MartiUK added a commit that referenced this pull request Dec 4, 2015
Added code to check for the existence of a customized ini file..
@MartiUK MartiUK merged commit 9a1d49b into cmderdev:development Dec 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants