Skip to content

New 'ledger-indent-region' function#399

Open
jdek wants to merge 1 commit into
ledger:masterfrom
jdek:region-indent
Open

New 'ledger-indent-region' function#399
jdek wants to merge 1 commit into
ledger:masterfrom
jdek:region-indent

Conversation

@jdek

@jdek jdek commented Feb 24, 2024

Copy link
Copy Markdown
  • ledger-mode.el (ledger-mode): Set indent-region-function.
  • ledger-post.el (ledger-indent-region): New function that indents the region by calling ledger-indent-line for each line.

Closes #197

* ledger-mode.el (ledger-mode): Set indent-region-function.
* ledger-post.el (ledger-indent-region): New function that indents the
region by calling ledger-indent-line for each line.

Closes bug ledger#197

Signed-off-by: J. Dekker <jdek@itanimul.li>
@jdek

jdek commented Mar 8, 2024

Copy link
Copy Markdown
Author

@purcell hi, possible to get a quick review on this? Would be quite nice to have this fixed in master.

@purcell

purcell commented Mar 11, 2024

Copy link
Copy Markdown
Member

Probably @enderw88 would be better placed to comment on whether this is a desirable fix — I'm not sure about the history of indentation in ledger-mode

@jdek

jdek commented Mar 31, 2024

Copy link
Copy Markdown
Author

Maybe @bcc32 could take a look?

@purcell

purcell commented Apr 1, 2024

Copy link
Copy Markdown
Member

As far as I can tell, the default indent-region-functionindent-region-line-by-line – calls indent-line-function on each line. So what's the difference here, assuming that indent-line-function is set to ledger-indent-line?

@purcell

purcell commented Apr 1, 2024

Copy link
Copy Markdown
Member

ie. what you've implemented feels the same as simply not setting indent-region-function in the first place.

@jdek

jdek commented Apr 1, 2024

Copy link
Copy Markdown
Author

Good catch, I wasn't aware that indent-region worked like that by default. Just unsetting indent-region-function seems to work for me.

Test input:

    2017-06-26 Commonplace Coffee
         Expenses:Restaurants:Coffee                 3.00   ; Grande
    Assets:Cash:Wallet                         -3.00

2017-06-26 Commonplace Coffee
       Expenses:Restaurants:Coffee                 3.00   ; Grande
    Assets:Cash:Wallet                         -3.00

          2017-06-26 Commonplace Coffee
      Assets:Cash:Wallet
        Expenses:Restaurants:Coffee                 3.00

Current master:

    2017-06-26 Commonplace Coffee
   Expenses:Restaurants:Coffee                 3.00   ; Grande
   Assets:Cash:Wallet                         -3.00

2017-06-26 Commonplace Coffee
   Expenses:Restaurants:Coffee                 3.00   ; Grande
   Assets:Cash:Wallet                         -3.00

   2017-06-26 Commonplace Coffee
   Assets:Cash:Wallet
   Expenses:Restaurants:Coffee                 3.00

With (setq-local indent-region-function nil):

2017-06-26 Commonplace Coffee
   Expenses:Restaurants:Coffee                 3.00   ; Grande
   Assets:Cash:Wallet                         -3.00

2017-06-26 Commonplace Coffee
   Expenses:Restaurants:Coffee                 3.00   ; Grande
   Assets:Cash:Wallet                         -3.00

2017-06-26 Commonplace Coffee
   Assets:Cash:Wallet
   Expenses:Restaurants:Coffee                 3.00

ledger-post-align-postings is broken somehow I guess?

@purcell

purcell commented Apr 1, 2024

Copy link
Copy Markdown
Member

ledger-post-align-postings is broken somehow I guess?

Hmmm... do we have any tests for that, perhaps? You could consider pushing a failing test to show what you'd expect.

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.

Can't indent regions

3 participants