Do not modify the buffer in `yaml-indent-line` unless necessary

[Hi-Angel]

Calling (yaml-indent-line) for the first time on a line that the function already deems indented correctly results in the buffer getting modified even though there was nothing to modify. This is because the function unconditionally removes space at the beginning of line and then re-indents text back.

Fix this by replacing the delete-horizontal-space + indent-to combination with a call to indent-line-to which does the right thing. While at it, factor out two calls to indent-to into just one indent-line-to

[Hi-Angel]

Hmm, actually I just realized this allows to remove (beginning-of-line) as well. Let me do that…

[Hi-Angel]

Done. Added a new commit as well.

[wasamasa]

@Hi-Angel I hate being the party pooper, but do you have a copyright assignment?

Regarding the code itself, I'd prefer if you'd stick to the original structure of (if (some-condition) (indent-line like-this) (indent-line like-that)).

[Hi-Angel]

Copyright assignment… with whom? I do have one with Emacs, but this code is a standalone plugin, do you require a separate copyright assignment for contribution?

[Hi-Angel]

Regarding the code itself, I'd prefer if you'd stick to the original structure of (if (some-condition) (indent-line like-this) (indent-line like-that)).

This is odd, but you are the maintainer, as you wish, I can fix that.

[wasamasa]

Regarding copyright assignment: See issue #44. Contributions from people without copyright assignment for Emacs would prevent inclusion of the package into GNU ELPA. That being said, the issue may be obsolete ever since the package was added to NonGNU ELPA. Also, yaml-ts-mode may entirely obsolete this package if tree-sitter becomes the preferred way of implementing major modes.

Regarding code style: No, it's not odd at all. My suggested change preserves the original structure of the code. Unless there is a compelling reason to move the if inside the indent-line call, it shouldn't be considered in the first place.

[Hi-Angel]

Regarding copyright assignment: See issue #44. Contributions from people without copyright assignment for Emacs would prevent inclusion of the package into GNU ELPA. That being said, the issue may be obsolete ever since the package was added to NonGNU ELPA.

Well, I do have copyright assignment and a number of commits to the upstream Emacs

Also, yaml-ts-mode may entirely obsolete this package if tree-sitter becomes the preferred way of implementing major modes.

Sounds interesting! Still, older Emacs versions I think will be around for a while even after tree-sitter gets released.

Regarding code style: No, it's not odd at all. My suggested change preserves the original structure of the code. Unless there is a compelling reason to move the if inside the indent-line call, it shouldn't be considered in the first place.

Well, if it's discussable, then the reason for the change is that it removes an excess call, so reduces the code. The style (foo (if (test) a b) is also functional (which LISP supposed to be), whereas the former style (if (test) (foo a) (foo b)) is imperative. Basically, the reason is that it's easier to read.

If the question is in regards to the change being inside a commit modifying something else, in this case I can move the change being discussed to its own commit.

[wasamasa]

Well, if it's discussable, then the reason for the change is that it removes an excess call, so reduces the code.

Small code size is not a goal, readability very much is.

The style (foo (if (test) a b) is also functional (which LISP supposed to be), whereas the former style (if (test) (foo a) (foo b)) is imperative.

Please do not spell Lisp in all-caps, this has been obsolete since the 70ies (unless you for some reason are programming in LISP 1.5, then please go ahead). If you're referring to contemporary Lisp implementations like Common Lisp and Emacs Lisp, the assertion that Lisp is supposed to be functional is plain wrong since they're multi-paradigm languages and usually written in an imperative style. Additionally, the argument that the former style is functional is wrong because functional programming is first and foremost about avoiding side effects, but the code still performs a side effect (changing the indentation) no matter whether you write it in the first or second style.

Basically, the reason is that it's easier to read.

I disagree for the previously mentioned reasons and ask you to change the code accordingly.

If the question is in regards to the change being inside a commit modifying something else, in this case I can move the change being discussed to its own commit.

No, this is not about it at all. It's about preserving the original code style.

[Hi-Angel]

Additionally, the argument that the former style is functional is wrong because functional programming is first and foremost about avoiding side effects, but the code still performs a side effect (changing the indentation) no matter whether you write it in the first or second style.

Alright, aaand, do you know how side-effects are being avoided…? :) I mean, you can't avoid them completely, they will be present disregarding whether you are coding in immutable and pure Haskell or in completely imperative C.

Well, you "avoid" them by separating side-effects and pure computation as much as possible. On that point, Wiki says, quoting:

[functional programming] is a declarative programming paradigm in which function definitions are trees of expressions that map values to other values, rather than a sequence of imperative statements which update the running state of the program.

So more computation with less mutation.

In the discussed context:

• the (foo (if (test) a b)) style has computation explicitly separated. The whole code inside (if …) expression is just calculation, whose result is then being passed to (foo …) call.
• the (if (test) (foo a) (foo b)) style mixes both calculation and state update, which is imperative style (and also more verbose).

Small code size is not a goal, readability very much is.

Sure! Well, you might say it's both less code and more readable in this case, so… 🤷‍♂️

No, this is not about it at all. It's about preserving the original code style.

Do you mean, style of the function yaml-indent-line or style of the file? Depending on which one you mean, I might have different answers.

I disagree for the previously mentioned reasons and ask you to change the code accordingly.

That… sounds imperative :) To reiterate, you are the maintainer, it is your project, which you maintain, not me; so if you require the change I will do that. It's just that you seemed to be open for discussion regarding the functional vs imperative change. But if you are keen to keep the old style disregarding the outcome of our dialog, I see no reason for the discussion at all.

[wasamasa]

Sigh. Closing this PR since you cannot take a hint at all.

[Hi-Angel]

lol. Are there any other maintainers that can intervene? @syohex perhaps? @yoshiki ?

I've no idea what @wasamasa is doing but closing a PR with explanation "you didn't guess my hint" is, well… ain't technical? 😄 I mean, this isn't serious, is this a joke? Can please somebody step in?

[wasamasa]

This is absolutely serious. How many more times do I have to repeat myself to show that? Rather than keeping this PR indefinitely open for unclear reasons, I'd rather close it until it's in a state I consider acceptable to merge.

[Hi-Angel]

This is absolutely serious. How many more times do I have to repeat myself to show that? Rather than keeping this PR indefinitely open for unclear reasons, I'd rather close it until it's in a state I consider acceptable to merge.

Well, maybe you should not use "hints" in conversation? 😂 I mean, seriously, I repeated twice that I will do what you want, just tell me that.

What did you do instead? You kept discussing the "functional vs imperative" style, and referring that as a reason you want some change. What should I, as a contributor, do? Participate of course 🤷‍♂️ Which I did. And then you suddenly closed the PR.

Let me repeat: I will change the style back if you want that. Like, what's the point of us discussing if you didn't want to discuss? 🤷‍♂️ You want the change — alright. Please reopen the PR, and I will edit the commits accordingly in no time.

[Hi-Angel]

So, FTR, I changed the commits. Anything else?

[wasamasa]

You forgot to push.

[Hi-Angel]

You forgot to push.

I think when PR is closed it doesn't update its state. To see the current commits in the branch it is needed to go the fork. Here's the 1st commit, you can see it was updated 6 minutes ago as of writing the words.

[wasamasa]

Good point. The "Reopen pull request" button is also greyed out for me, so you'd have to open a new PR.

[Hi-Angel]

I see, np, will do