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 A sub-menu to change the types of Font and Font-Styles #25

Merged
merged 16 commits into from
Mar 15, 2023

Conversation

BarmenduC
Copy link
Contributor

A Menu has been added into the already existing context menu listing various font-families and font-styles that can be used in the note. This caters to #8 and partially to #9 . A few fixes still need to be carried out (would probably be working upon that shortly) .

Copy link
Member

@humdingerb humdingerb left a comment

Choose a reason for hiding this comment

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

  • Please use the haiku coding style, as does the rest of the code. There's haiku-format in HaikuDepot that can help with that. I commented a few things, but there are more violations. Also remove additionally inserted newlines.
  • The current font menu items aren't checkmarked.
  • The replicated view doesn't show the text with the set font.
  • Please squash related commits before submitting a PR.

DeskNoteView.cpp Show resolved Hide resolved
DeskNoteView.cpp Outdated Show resolved Hide resolved
DeskNoteView.cpp Outdated Show resolved Hide resolved
DeskNoteView.cpp Outdated Show resolved Hide resolved
DeskNoteView.cpp Outdated Show resolved Hide resolved
DeskNoteView.cpp Outdated Show resolved Hide resolved
DeskNoteView.cpp Outdated Show resolved Hide resolved
DeskNoteView.cpp Outdated Show resolved Hide resolved
@BarmenduC
Copy link
Contributor Author

  • Please use the haiku coding style, as does the rest of the code. There's haiku-format in HaikuDepot that can help with that. I commented a few things, but there are more violations. Also remove additionally inserted newlines.
  • The current font menu items aren't checkmarked.
  • The replicated view doesn't show the text with the set font.
  • Please squash related commits before submitting a PR.

Before performing the code-style cleanup, I'll read up the haiku coding style completely. Would perform the clean up soon enough.

Just to make sure, the tab style for variable declaration is to be dropped everywhere (even in header files) or just inside this file?

The stuff that I mentioned that is still remaining to be fixed are namely these two -

  1. Changed Font not appearing in replicants
  2. Checkmark not appearing in-front of selected font

I had planned to keep working upon these issues after this PR gets merged, and to mention these 2 as separate issues so that, in case I am incapable of making it work, someone else can take up the work later. Would that be okay?

@humdingerb
Copy link
Member

Just to make sure, the tab style for variable declaration is to be dropped everywhere (even in header files) or just inside this file?

Just in the cpp file you changed.

  1. Changed Font not appearing in replicants
  2. Checkmark not appearing in-front of selected font

I had planned to keep working upon these issues after this PR gets merged, and to mention these 2 as separate issues so that, in case I am incapable of making it work, someone else can take up the work later. Would that be okay?

I suppose so.
BTW, I recommend getting familiar with using 'git' from the commandline and using feature branches (see https://github.com/haikuports/haikuports/wiki/DevelopmentModel for an explanation). It's much more powerful and even convenient, once you get used to it.

@BarmenduC
Copy link
Contributor Author

Requested changes have been mostly fixed. Will be keeping my out for more code-style fixes while I keep working upon this further.

@BarmenduC BarmenduC requested a review from humdingerb March 14, 2023 01:12
Copy link
Member

@humdingerb humdingerb left a comment

Choose a reason for hiding this comment

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

Please have another look at the coding style at https://www.haiku-os.org/development/coding-guidelines . As mentioned, "haiku-format" from HaikuDepot can help you.

  • Curly brackets at the end of "switch" statements
  • Same for "for" and "if" conditions
  • Space after "for", "if" etc.
  • Space around operators ( =, >= etc)

DeskNoteView.cpp Outdated Show resolved Hide resolved
DeskNoteView.cpp Outdated Show resolved Hide resolved
DeskNoteView.cpp Outdated Show resolved Hide resolved
@BarmenduC BarmenduC requested a review from humdingerb March 14, 2023 23:30
@BarmenduC
Copy link
Contributor Author

Manual code-style-fixes performed and .haiku-format added

Copy link
Member

@humdingerb humdingerb left a comment

Choose a reason for hiding this comment

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

Much better. :)
Please remove the haiku.format file. It's only needed somewhere in the folder tree when executing the haiku-format tool.
There are quite a few stray whitespaces around. I recommend installing Koder and "Highlight trailing whitespaces" in its settings. Then they are easily to spot. Maybe even try "Trim trailing whitespaces" from its "Edit" menu.

DeskNoteView.cpp Outdated Show resolved Hide resolved
DeskNoteView.cpp Outdated Show resolved Hide resolved
@BarmenduC
Copy link
Contributor Author

Changes performed :-

  1. Removed .haiku-format
  2. Trimmed trailing whitespaces using Koder
  3. Added the missing opening bracket
  4. Removed newlines after case statements

BTW, I had a query -> How did the Haiku developers settle on this codestyle (If this is not the right place to ask stuff like this, do tell me where I can do that)(IRC maybe)

@BarmenduC BarmenduC requested a review from humdingerb March 15, 2023 16:30
@humdingerb
Copy link
Member

BTW, I had a query -> How did the Haiku developers settle on this codestyle (If this is not the right place to ask stuff like this, do tell me where I can do that)(IRC maybe)

Well, when many people work on a large project for a long time, there needs to be a style guide. The main developers agreed upon the rules very quickly and it changed only very little after that. While the HaikuArchives projects aren't strictly held to these style rules, it is a huge advantage to try have everyone adopt it. Makes it easy to contribute to any project without having to relearn a new style, or god-forbid,everyone doing their own thing and the code becoming a huge smorgasbord of styles...

I'll merge this for now. If it turns out you can't continue working on the open issues, and nobody else does, we can revert the changes.

In the future, please use a feature branch and squash your commits to sensible code bites.

@humdingerb humdingerb merged commit f0185a1 into HaikuArchives:master Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants