Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix #2659: Use get_hg_branch() to get Mercurial branch information.#2660

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
daxgames merged 1 commit intocmderdev:masterfromvsajip:fix-2659
Feb 3, 2022

Conversation

@vsajip
Copy link
Contributor

No description provided.

@daxgames
Copy link
Member

daxgames commentedJan 29, 2022
edited
Loading

@vsajip can you please take a screenshot and paste it into a comment so we can see the prompt before and after your modification?

I not use mercurial so I have no way to test.

@vsajip
Copy link
ContributorAuthor

vsajip commentedJan 29, 2022
edited
Loading

I can do that, but the prompt hasn't actually changed - only the internal implementation of it has. The original code directly did anio.popen inhg_prompt_filter(). The updated code callsget_hg_branch(), which does theio.popen.

@vsajip
Copy link
ContributorAuthor

ss3

In the above screenshot, the last line shows how the prompt looks in a directory with a project using Mercurial as the VCS.

@daxgames
Copy link
Member

daxgames commentedJan 30, 2022
edited
Loading

@vsajip I am not trying to be difficult but why change it then?

What is the benefit of your 'fix' over what the current code does if the final result does not change?

Just trying to understand.

@vsajip
Copy link
ContributorAuthor

Well,clink.lua contains functions such asget_svn_dir(),get_svn_branch(),get_git_dir(),get_git_branch(),get_hg_dir(),get_hg_branch() for all the different VCS types it supports. These functions were originally written to provide specific information - in the case ofget_hg_branch(), it is there to provide Mercurial branch information. I have no idea why that function was bypassed, and whether the change in this PR should be made or not depends on how one values consistency and maintainability in assessing software quality. Clearly "it works" is one (perhaps the minimum) level, but one can also aspire to those other qualities I mentioned. To me, it is better to have the consistency of approach across all VCSes rather than the current situation around bypassingget_hg_branch().

DRSDavidSoft reacted with thumbs up emoji

@daxgames
Copy link
Member

I agree completely was just wondering if you were trying to fix an issue you saw.

Thamks for the PR

@vsajip
Copy link
ContributorAuthor

You're welcome. Thanks for your part in maintaining cmder, it makes the Windows command line so much better! I have some local changes to my cmder setup, and every time I update cmder, I need to retrofit those changes into my newly updated cmder. One of those is support for Mercurial and I came across this when doing my retrofit after installing cmder 1.3.19.

There are some other cosmetic things that I do (e.g. spaces between the(branch) and the rest of the prompt, and placement and formatting of the Python virtual environment name in the prompt). I don't know if the cmder dev team would be interested in those. I also have some ideas about making the configuration a bit more data driven, so that certain elements could be picked up from configuration files instead of being hardcoded in Lua scripts. I'd be interested in your thoughts ...

@daxgames
Copy link
Member

@vsajip v1.3.19 has support for a configurable prompt.

I am curious to know what you mean by retrofitting after upgrade. What kind of changes are you making and how do they change the appearance of the shell?

I am wondering if your retrofits could be made configurable items in newer Cmder versions.

We are always interested in user contribution but we need to be careful not to undo something current users depend on.

@vsajip
Copy link
ContributorAuthor

What kind of changes are you making and how do they change the appearance of the shell?

No changes to the appearance of the shell. The two main changes are:

  • The prompt itself (I customise the exact prompt that I want - so single line, no lambda character etc.)
  • I change theget_hg_branch() function to callhg prompt - an open source Mercurial extension I have installed - which provides better branch information than just "hg branch" as used in the vanillaget_hg_branch().

I am wondering if your retrofits could be made configurable items in newer Cmder versions.

Definitely. IMO both the prompt itself and the command to get branch information could be made configurable. On Linux I use a project calledbash_git_prompt to provide an enhanced prompt for when you're under a Git project directory. I feel cmder could use some of that project's approach to enhance the Git information provided.

We are always interested in user contribution but we need to be careful not to undo something current users depend on.

Absolutely, I understand the need for backward compatibility. I'm a committer on the Python project, which takes it very seriously, and we sometimes have to reject suggested improvements where they can't be made in a backwards-compatible way.

@vsajip
Copy link
ContributorAuthor

v1.3.19 has support for a configurable prompt.

If you meancmder_prompt_config.lua, I'm aware of it.

@daxgames
Copy link
Member

  • The prompt itself (I customise the exact prompt that I want - so single line, no lambda character etc.)

The above and more is already possible for Cmder v1.3.19Cmd.exe prompts. Seethis or go straight to the config file%cmder_root%/config/cmder_prompt_config.lua.

@vsajip
Copy link
ContributorAuthor

Thanks, I'm using that file now. I'll look to see how I can minimise any retrofitting I need to do in this area.

@daxgamesdaxgames merged commit0616ff0 intocmderdev:masterFeb 3, 2022
@daxgames
Copy link
Member

Thanks - Merged

vsajip reacted with thumbs up emoji

@vsajipvsajip deleted the fix-2659 branchFebruary 4, 2022 02:44
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vsajip@daxgames

[8]ページ先頭

©2009-2025 Movatter.jp