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

Add support for setting console emulator font name #9332

Conversation

dasiths
Copy link
Contributor

@dasiths dasiths commented Jul 4, 2021

Add Support For Setting Console Emulator Font Name

This is the 2nd part supplementing the PR to the ConEmu fork gitextensions/conemu-inside#28. The idea is to allow powerline supported fonts.

As shown on the screenshot below, setting the font name to a powerline supported font resulted in this nice rendering.
image

image

I could not get the design view for ConsoleStyleSettingsPage to load in Visual Studio 2019 but I hand edited the ConsoleStyleSettingsPage.designer.cs file directly. It build, runs and renders fine but as an improvement I would like to use the standard button based font selection and combine font name + size. I'll do it as a separate PR next.

✒️ I contribute this code under The Developer Certificate of Origin.

@RussKie
Copy link
Member

RussKie commented Jul 4, 2021

Please make sure the solution builds.

C:\Program Files\dotnet\sdk\5.0.301\Microsoft.Common.CurrentVersion.targets(1717,5): warning NU1702: ProjectReference 'C:\projects\gitextensions\Externals\ICSharpCode.TextEditor\Project\ICSharpCode.TextEditor.csproj' was resolved using '.NETFramework,Version=v4.6.1' instead of the project target framework '.NETCoreApp,Version=v5.0'. This project may not be fully compatible with your project. 

@RussKie RussKie added the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Jul 4, 2021
@mstv
Copy link
Member

mstv commented Jul 4, 2021

Please make sure the solution builds.

It might be caused by the changed other submodules. Perhaps the merge went wrong.

Basically, please do not merge anything into feature branches - rebase instead.

@ghost ghost removed the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Jul 5, 2021
@dasiths
Copy link
Contributor Author

dasiths commented Jul 5, 2021

@RussKie @mstv I've reverted the changes for other submodules. It builds locally for both previously and also now.

Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

Build FAILED.
C:\projects\gitextensions\GitExtensions\Project.Loc.targets(89,5): error : Please update English translations and re-submit the pull-request. Refer to https://github.com/gitextensions/gitextensions/wiki/Translations [C:\projects\gitextensions\GitExtensions\GitExtensions.csproj]
    0 Warning(s)
    1 Error(s)

@ghost ghost added 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity and removed 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity labels Jul 5, 2021
@dasiths

This comment has been minimized.

Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

This is radical, but I think this is a good direction. However now the "Console style" page is pretty empty, and perhaps we could consider moving the console font setting to the "Fonts" page, and the console style dropdown to the "Colors" page (under its own group, like "Console theme"?).
What do you think?

Comment on lines 8708 to 8715
<file datatype="plaintext" original="Plugin" source-language="en">
<body>
<trans-unit id="Description.Text">
<source>Plugin Manager</source>
<target />
</trans-unit>
</body>
</file>
Copy link
Member

Choose a reason for hiding this comment

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

Please revert this piece

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is radical, but I think this is a good direction. However now the "Console style" page is pretty empty, and perhaps we could consider moving the console font setting to the "Fonts" page, and the console style dropdown to the "Colors" page (under its own group, like "Console theme"?).
What do you think?

I could be awayed either way. I like that the console related stuff is in one sub page but I get your thinking as well.

I was contemplating the fact that the entire ConEmu settings xml can be loaded externally as well and that maybe another feature I could work on next. If we want to try that, then we would need the console sub page to place the xml file path picker for example.

i.e. These are all the settings embedded in that xml file
image

This isn't a reason to keep the console sub page though. Just something that came to my mind. Let me know how you want to proceed. I can go with you suggested approach if you want.

Copy link
Member

Choose a reason for hiding this comment

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

I was contemplating the fact that the entire ConEmu settings xml can be loaded externally as well and that maybe another feature I could work on next. If we want to try that, then we would need the console sub page to place the xml file path picker for example.

i.e. These are all the settings embedded in that xml file
image

Instead of duplicating the ConEmu's settings page, I'd rather see us providing a mechanism to launch the ConEmu configurator. Though personally I don't think it adds any value, you're one of a handful contributors who expressed the desire to tweak it.

Copy link
Contributor Author

@dasiths dasiths Jul 6, 2021

Choose a reason for hiding this comment

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

Yeah I wouldn't wanna duplicate it. Rather I was thinking let us point to a xml file from our settings. But like you said, it might be overkill.

So provided we want to continue with the reorg of pages, should I tackle the sub page removal/reorg as a separate PR?

@dasiths
Copy link
Contributor Author

dasiths commented Jul 5, 2021

Build FAILED.
C:\projects\gitextensions\GitExtensions\Project.Loc.targets(89,5): error : Please update English translations and re-submit the pull-request. Refer to https://github.com/gitextensions/gitextensions/wiki/Translations [C:\projects\gitextensions\GitExtensions\GitExtensions.csproj]
    0 Warning(s)
    1 Error(s)

I've added the translations but the build still keep failing :(

@mstv
Copy link
Member

mstv commented Jul 5, 2021

I've added the translations but the build still keep failing :(

This is an open issue if the master has translation changes.
So it is a perfect time to rebase on master and squash.

@RussKie
Copy link
Member

RussKie commented Jul 6, 2021

This is an open issue if the master has translation changes.

I don't believe this has anything to do with #9273

This is what's causing the issue:
image
image

@RussKie RussKie added the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Jul 6, 2021
@dasiths
Copy link
Contributor Author

dasiths commented Jul 6, 2021

This is an open issue if the master has translation changes.

I don't believe this has anything to do with #9273

This is what's causing the issue:
image
image

But that's the change you wanted me to revert right? It fails translation before and after reverting that change.

@ghost ghost removed the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Jul 6, 2021
@RussKie
Copy link
Member

RussKie commented Jul 6, 2021

Yes. You're introduced trailing spaces, which must be undone for the translation check to pass.

@dasiths dasiths force-pushed the dasithw/git-extensions-console-font-name-support branch 2 times, most recently from e340f5e to 07319d8 Compare July 6, 2021 05:22
@dasiths
Copy link
Contributor Author

dasiths commented Jul 6, 2021

Yes. You're introduced trailing spaces, which must be undone for the translation check to pass.

Fixed.

@RussKie
Copy link
Member

RussKie commented Jul 7, 2021

Please provide an updated screenshot

Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

👍
Please squash

@RussKie RussKie added the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Jul 7, 2021
@ghost ghost removed the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Jul 7, 2021
@dasiths dasiths force-pushed the dasithw/git-extensions-console-font-name-support branch from 07319d8 to 4d33064 Compare July 7, 2021 06:53
@dasiths
Copy link
Contributor Author

dasiths commented Jul 7, 2021

👍
Please squash

@RussKie done and pushed.

@RussKie RussKie merged commit 06a9deb into gitextensions:master Jul 7, 2021
@ghost ghost added this to the 3.6 milestone Jul 7, 2021
@RussKie
Copy link
Member

RussKie commented Jul 7, 2021

Thank you

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.

None yet

3 participants