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

Bug799430 Split auto complete bug #2031

Merged
merged 2 commits into from
Oct 21, 2024
Merged

Conversation

Bob-IT
Copy link
Contributor

@Bob-IT Bob-IT commented Oct 11, 2024

I think the first commit fixes the issue that if you try to auto complete a split using the memo text from a split that has no values it fails to fill in the transfer account. This has stemmed from a change in the return value of xaccSplitSetSharePrice in version 4.10

Just looking for confirmation of change before I push this.

The second commit is just realigning the source file after the change to cpp.

Copy link
Member

@jralls jralls left a comment

Choose a reason for hiding this comment

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

The first commit makes sense and is fine. The spacing one needs some QA. What tool did you use for indentation and spaces-before-braces?

gnucash/register/ledger-core/split-register-control.cpp Outdated Show resolved Hide resolved
gnucash/register/ledger-core/split-register-control.cpp Outdated Show resolved Hide resolved
gnucash/register/ledger-core/split-register-control.cpp Outdated Show resolved Hide resolved
gnucash/register/ledger-core/split-register-control.cpp Outdated Show resolved Hide resolved
gnucash/register/ledger-core/split-register-control.cpp Outdated Show resolved Hide resolved
gnucash/register/ledger-core/split-register-control.cpp Outdated Show resolved Hide resolved
gnucash/register/ledger-core/split-register-control.cpp Outdated Show resolved Hide resolved
gnucash/register/ledger-core/split-register-control.cpp Outdated Show resolved Hide resolved
@Bob-IT
Copy link
Contributor Author

Bob-IT commented Oct 17, 2024

The first commit makes sense and is fine. The spacing one needs some QA. What tool did you use for indentation and spaces-before-braces?

No tool used, I just find it easier to read the source files if there is a consistent use of spaces and just changed it visually.

@jralls
Copy link
Member

jralls commented Oct 17, 2024

No tool used, I just find it easier to read the source files if there is a consistent use of spaces and just changed it visually.

Using a tool is easier. We've used astyle in the past, usually with args --indent=spaces=4 --brackets=break --suffix=none though the last one I did was -xd -k1 -m0 -M60 -xL -xC79 -OHpUcZns4 --brackets=break. That was on the SQL backend before I started the C++ rewrite. Phil Longstaff had a rather unique formatting style and it needed a lot of adjustments.

Since you did it by eye, a question: Why do you like a space between a function name and its opening parenthesis but not between a preprocessor macro and its opening parenthesis?

@jralls
Copy link
Member

jralls commented Oct 18, 2024

Go ahead and merge when you're ready.

When using split autocomplete, the transfer account was not being set
due to a change in xaccSplitGetSharePrice in 4.10 that return 0 instead
of 1 when there is no price so reflect this change in the following
function gnc_find_split_in_trans_by_memo
@code-gnucash-org code-gnucash-org merged commit e63210f into Gnucash:stable Oct 21, 2024
4 checks passed
@Bob-IT
Copy link
Contributor Author

Bob-IT commented Oct 21, 2024

Will install astyle and do some testing.

Since you did it by eye, a question: Why do you like a space between a function name and its opening parenthesis but not between a preprocessor macro and its opening parenthesis?

If I remember correctly, in the early days when I was submitting patches on bug reports it was suggested to do it that way.

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.

3 participants