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

Indent lines after hanging operator even if previous line contains # #115

Merged
merged 4 commits into from
Jul 17, 2020

Conversation

non-Jedi
Copy link
Contributor

@non-Jedi non-Jedi commented Apr 1, 2020

Because the previous implementation used a regex to detect whether the
hanging operator was in a comment or not, it did not detect hanging
operators following a # in a string.

Fixes #19

This also fixes an unrelated issue where the following didn't get properly indented. Haven't dug in to see what the underlying problem was. That's why the unrelated test had to be changed; the regex no longer applied since the buffer was getting properly indented.

f(x) =
x*
x

@non-Jedi
Copy link
Contributor Author

non-Jedi commented Apr 1, 2020

Please only review the last commit. This PR is based on #114 since it needs 8b37429 to byte-compile cleanly.

non-Jedi added 3 commits July 13, 2020 17:35
These are changes unrelated to JuliaEditorSupport#15 that just make the code a bit more
readable. This commit should be reviewed separately.

Since the julia group was declared with a :prefix of julia- all of
these customization variables will be in the julia group anyway. Also,
since julia-mode didn't declare it's group, an anonymous julia-mode
group was getting created for faces.
Because the previous implementation used a regex to detect whether the
hanging operator was in a comment or not, it did not detect hanging
operators following a # in a string.

Fixes JuliaEditorSupport#19
@non-Jedi
Copy link
Contributor Author

non-Jedi commented Jul 14, 2020

@giordano or @FelipeLema could you please review this when you get a chance? Or @rfourquet if you're feeling up to it?

Copy link
Contributor

@FelipeLema FelipeLema left a comment

Choose a reason for hiding this comment

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

good refactor. thanks for taking the time to debug this one

Copy link
Collaborator

@rfourquet rfourquet left a comment

Choose a reason for hiding this comment

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

Nice improvement!

@@ -388,7 +389,7 @@ symbol, gives up when this is not true."
(while (and (not done) (< (point-min) (point)))
(julia-safe-backward-sexp)
(cond
((looking-at (rx (or "import" "export" "using")))
((looking-at (regexp-opt (list "import" "export" "using")))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why prefering regexp-opt to rx?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If I remember correctly regexp-opt is more efficient if it is applicable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my justification, but actually trying the functions, they generate the same regex strings in this case. I can revert if desired.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No need, it is perfect as is.

Copy link
Collaborator

@tpapp tpapp left a comment

Choose a reason for hiding this comment

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

Very nice work, thanks! And very clean elisp code.

julia-mode.el Show resolved Hide resolved
julia-mode.el Show resolved Hide resolved
@@ -388,7 +389,7 @@ symbol, gives up when this is not true."
(while (and (not done) (< (point-min) (point)))
(julia-safe-backward-sexp)
(cond
((looking-at (rx (or "import" "export" "using")))
((looking-at (regexp-opt (list "import" "export" "using")))
Copy link
Collaborator

Choose a reason for hiding this comment

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

If I remember correctly regexp-opt is more efficient if it is applicable.

@tpapp tpapp merged commit b5f5983 into JuliaEditorSupport:master Jul 17, 2020
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.

indentation with the comment characters and the pipe operator
4 participants