ledger / ledger-mode

Emacs Lisp files for interacting with the C++Ledger accounting system
GNU General Public License v2.0
381 stars 76 forks source link

trailing whitespace not removed when prefilling account name in minibuffer #28

Open herrsimon opened 7 years ago

herrsimon commented 7 years ago

Hello,

if the cursor is on an account line which has some amount and 'ledger-report', 'ledger-reconcile' or any other function which needs an account name as input is run, the account the cursor is on is pasted into the minibuffer including trailing whitespace, which makes the preselection useless. As an example, consider the following transaction (square brackets indicate cursor position):

2017-01-01 sample transaction
    expenses:someexpe[n]se    10.00
    accounts:payable

When running 'ledger-report' and then choosing 'balance', the prefilled account name is 'expenses:someexpense '. As of course there is no such account, I have to reenter the account name without the trailing spaces manually.

I looked at the code a bit but couldn't find the problem (don't know much elisp), I guess it's a simple removal of trailing whitespace somewhere in the internal context providing functions.

Could you please fix this?

Thanks a lot,

Simon

enderw88 commented 7 years ago

I do not have this problem. I would have noticed this many years ago. Please give me deatils on what emacs you are using and what other modes you have working.

On Tue, Mar 21, 2017 at 6:05 PM, herrsimon notifications@github.com wrote:

Hello,

if the cursor is on an account line which has some amount and 'ledger-report', 'ledger-reconcile' or any other function which needs an account name as input is run, the account the cursor is on is pasted into the minibuffer including trailing whitespace, which makes the preselection useless. As an example, consider the following transaction (square brackets indicate cursor position):

2017-01-01 sample transaction expenses:someexpe[n]se 10.00 accounts:payable

When running 'ledger-report' and then choosing 'balance', the prefilled account name is 'expenses:someexpense '. As of course there is no such account, I have to reenter the account name without the trailing spaces manually.

I looked at the code a bit but couldn't find the problem (don't know much elisp), I guess it's a simple removal of trailing whitespace somewhere in the internal context providing functions.

Could you please fix this?

Thanks a lot,

Simon

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ledger/ledger-mode/issues/28, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1rLOvmfqpP_QUjx4OnGRY-13Ityiz8ks5roHPMgaJpZM4MkmhF .

-- Craig, Corona De Tucson, AZ [image: missile_flyout] enderw88.wordpress.com

herrsimon commented 7 years ago

I'm using Emacs 25.1.1 but the problem is actually present since a long time. Disabling all minor modes doesn't fix it either. I investigated a bit further: The problem is with ledger-extract-context-info, more specifically the regular expressions in the variable ledger-line-config. It actually only occurs when specifying a commodity after the amount. To elaborate on the example from above: If I run ledger-report on a ledger file containing nothing but the transaction (again square brackets indicate cursor position)

2017-01-01 sample transaction
     expenses:someexpe[n]se          10.00
     accounts:payable

then ledger-extract-context-info returns

(acct-transaction account ((indent "     " 31) (status "" 36) (account "expenses:someexpense" 36) (separator "        " 60) (commodity "10.00" 68) (amount nil nil)))

If the transaction reads

2017-01-01 sample transaction
     expenses:someexpe[n]se          10.00 EUR
     accounts:payable

the return value is

(acct-transaction account ((indent "     " 31) (status "" 36) (account "expenses:someexpense        " 36) (separator "10.00" 68) (amount " " 73) (commodity nil nil)))

Increasing the amount of whitespace between the amount (10.00) and the commodity (EUR) doesn't change anything. So I guess ledger-mode assumes that the commodity always has to come before the amount and this is where the problem lies. Maybe first checking for existence and relative position of a commodity and then conditionally loading regular-expression tables is the right thing to do.

enderw88 commented 7 years ago

I see it now. So, 5 years after the majority of that code has been working fine we find a fundamental bug. I hate it when that happens. I will take a look at it and try to fix before next week.

On Wed, Mar 22, 2017 at 1:36 AM, herrsimon notifications@github.com wrote:

I'm using Emacs 25.1.1 but the problem is actually present since a long time. Disabling all minor modes doesn't fix it either. I investigated a bit further: The problem is with ledger-extract-context-info, more specifically the regular expressions in the variable ledger-line-config. It actually only occurs when specifying a commodity after the amount. To elaborate on the example from above: If I run ledger-report on a ledger file containing nothing but the transaction (again square brackets indicate cursor position)

2017-01-01 sample transaction expenses:someexpe[n]se 10.00 accounts:payable

then ledger-extract-context-info returns

(acct-transaction account ((indent " " 31) (status "" 36) (account "expenses:someexpense" 36) (separator " " 60) (commodity "10.00" 68) (amount nil nil)))

If the transaction reads

2017-01-01 sample transaction expenses:someexpe[n]se 10.00 EUR accounts:payable

the return value is

(acct-transaction account ((indent " " 31) (status "" 36) (account "expenses:someexpense " 36) (separator "10.00" 68) (amount " " 73) (commodity nil nil)))

Increasing the amount of whitespace between the amount (10.00) and the commodity (EUR) doesn't change anything. So I guess ledger-mode assumes that the commodity always has to come before the amount and this is where the problem lies. Maybe first checking for existence and relative position of a commodity and then conditionally loading regular-expression tables is the right thing to do.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ledger/ledger-mode/issues/28#issuecomment-288331611, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1rLIbDD7bEPL1pnp2JhQtXXXe5bdVKks5roN2TgaJpZM4MkmhF .

-- Craig, Corona De Tucson, AZ [image: missile_flyout] enderw88.wordpress.com

herrsimon commented 7 years ago

Nobody's perfect and besides, both ledger and ledger-mode are great pieces of software that not only have drastically simplified all my bookkeeping needs but also provided me with much better insight! I made the switch about two years ago and could replace three bloated and partially closed-source gui-applications by a single open-source command-line tool, a very liberating experience... Thanks a lot for that and also for fixing the bug!

enderw88 commented 7 years ago

I have been working on this. The fix is much trickier than it should be.

On Wed, Mar 22, 2017 at 07:13 herrsimon notifications@github.com wrote:

Nobody's perfect and besides, both ledger and ledger-mode are great pieces of software that not only have drastically simplified all my bookkeeping needs but also provided me with much better insight! I made the switch about two years ago and could replace three bloated and partially closed-source gui-applications by a single open-source command-line tool, a very liberating experience... Thanks a lot for that and also for fixing the bug!

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/ledger/ledger-mode/issues/28#issuecomment-288410940, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1rLPj0nkHYCVIJiVMMJmwKtqFXPnflks5roSxvgaJpZM4MkmhF .

-- Craig, Corona De Tucson, AZ [image: missile_flyout] enderw88.wordpress.com

doolio commented 9 months ago

As this been resolved? Running the latest ledger-mode and I don't encounter this problem. If so then I think we can close this issue.

enderw88 commented 9 months ago

Zombie problem. I don't remember fixing it, but nearly seven years have passed.

On Thu, Jan 4, 2024 at 6:34 AM doolio @.***> wrote:

As this been resolved? Running the latest ledger-mode and I don't encounter this problem. If so then I think we can close this issue.

— Reply to this email directly, view it on GitHub https://github.com/ledger/ledger-mode/issues/28#issuecomment-1877104226, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGWWLCSKDBG7KMNU4AXE2DYM2VXPAVCNFSM4DESNBC2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBXG4YTANBSGI3A . You are receiving this because you commented.Message ID: @.***>

-- Craig, Corona De Tucson, AZ [image: missile_flyout] enderw88.wordpress.com

doolio commented 9 months ago

Right, so I think we can close this issue. If it truly still exists then someone can report it anew or have this issue re-opened.