Package: emacs;
Reported by: Artem <snake05865 <at> gmail.com>
Date: Sat, 28 Dec 2024 04:38:02 UTC
Severity: minor
Found in version 31.0.50
Message #22 received at 75154 <at> debbugs.gnu.org (full text, mbox):
From: Yuan Fu <casouri <at> gmail.com> To: Artem <snake05865 <at> gmail.com> Cc: 75154 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, Theodor Thornhill <theo <at> thornhill.no>, Stefan Kangas <stefankangas <at> gmail.com> Subject: Re: bug#75154: 31.0.50; java-ts-mode. Issues with Indentation Date: Thu, 13 Feb 2025 22:24:06 -0800
[Message part 1 (text/plain, inline)]
> On Jan 28, 2025, at 6:06 AM, Artem <snake05865 <at> gmail.com> wrote:
>
> Okay, you may personally disagree. But why not create a separate
> setting tailored to your preferences? You seem to agree that IntelliJ IDEA
> is currently the canonical editor for Java. Set 8 spaces as the default and
> add an option to adjust it.
>
> Do I need to provide examples of major open-source projects that use 8 spaces?
> For instance, Apache Tomcat:
> https://github.com/apache/tomcat/blob/cd997770105ea960764c3d7844e13c0bbe8fd3c1/java/org/apache/juli/OneLineFormatter.java#L102
> Or Red Hat Quarkus:
> https://github.com/quarkusio/quarkus/blob/0e013fabfc14557e688b559f6edea43b8934406a/devtools/cli/src/main/java/io/quarkus/cli/CliPlugins.java#L37
>
> I think this is a very important matter. Personally,
> it drives me crazy when the spacing is off.
>
>> While this is probably catchable, the "workaround" I started using is to
>> end the statement with a semi, then RET:
>>
>> ```
>> class Foo {
>> void Foo() {
>> List<Customer> customers = customer
>> |; // Point is now at |
>>
>> }
>> }
>> ```
>>
>> Now we have a valid parse tree, indented correctly, and can continue
>> chaining. I agree that this very much is a hack.
>
> Yes, it does look unnatural. And you know, there’s a similar issue in bash-ts-mode.
> I can’t show the exact code snippet right now, but the behavior is essentially the same.
> You press RET, the cursor ends up in the wrong position, then press RET again,
> and the code block jumps to where it’s supposed to be.
> Where’s the error? Is it a problem with the mode’s implementation
> or a bug in Tree-sitter itself? Or did everyone just borrow logic from
> each other, and now this indentation behavior has spread to other modes?
>
>>
>>> Moreover, the current indentation is hardcoded and doesn’t allow
>>> flexibility. For instance, in python-ts-mode, pressing TAB allows you to
>>> adjust constructions more freely.
>>> This level of flexibility would be beneficial in this context
>>>
>>> This inflexibility prevents writing common patterns like:
>>> @Override
>>> protected void configure(HttpSecurity http) throws Exception {
>>> http
>>> .authorizeRequests()
>>> .antMatchers("/admin/**").hasAuthority("ADMIN")
>>> .antMatchers("/user").hasAnyAuthority("USER", "ADMIN")
>>> .antMatchers("/", "/index").permitAll()
>>> Example 2:
>>>
>>> The following looks correct -
>>>
>>> public class FloodFill {
>>> public static void main(String[] args) {
>>> List<Foo> stream = students.stream()
>>> .filter(item -> {
>>> return item.getValue() > 100 &&
>>> item.isActive();
>>> })
>>> .map()
>>> .collect();
>>> }
>>> }
>>>
>>> But java-ts-mode produces:
>>>
>>> public class FloodFill {
>>> public static void main(String[] args) {
>>> List<Foo> stream = students.stream()
>>> .filter(item -> {
>>> return item.getValue() > 100 &&
>>> item.isActive();
>>> })
>>
>> Yes, this really annoys me too, so I should try to fix this. I don't
>> remember from the top of my head what the problem was, but it forced me
>> either to omit the curlies, or just extract the body into a function and
>> pass it. But we should absolutely fix this. I believe there were several
>> difficulties here, where some are related to incomplete statements, and
>> some is related to treesit.el and the java parser idiosyncrasies. But
>> this is a valid bug in an of itself. If not too much of a hassle, could
>> you create a separate bugreport just for this case?
>>
>>> .map()
>>> .collect();
>>> }
>>> }
>
> I’m not sure. Maybe I can create a new bug report? But honestly,
> I don’t understand the point. A new report just to track the resolution
> of some minor issue that annoys you and that you consider more important than others?
>
>>>
>>> 2) Inner Classes
>>> Example 1:
>>> public class Outer {
>>> class Inner {| <-- cursor here moves Inner class unexpectedly
>>>
>>> }
>>> }
>>> Example 2:
>>> public class Outer {
>>> class Inner { // ???
>>> | <-- cursor here.
>>> }
>>> }
>>>
>>> Why does this happen? I did not request this behavior. While Example 1
>>> demonstrates bad code style, it is technically valid. Such "magical"
>>> formatting should be handled by a Java formatter, not by Emacs or
>>> Tree-sitter rules.
>>> IntelliJ IDEA does not apply such formatting; it leaves this task to the formatter.
>>>
>>
>> This is due to electric indent. You could disable it yourself in your
>> config or just live with it.
>
> Yes, I’ve already figured that out, thank you. But what does it give me? Am I supposed
> to just live with it? By the way, I tried disabling electric-indent-mode, but then indentation
> stopped working entirely. I briefly went through the documentation to understand what’s going on,
> and from what I gathered, this mode uses various backends, including Tree-sitter now.
>
> I could be wrong, but it seems like electric-indent-mode needs a review and a rethinking
> of how it should work, considering Tree-sitter is being actively used.
>
> And the extra “magic” performed by electric-indent-mode might have been relevant a long
> time ago, or maybe it still is in some cases. But for Java/C/C++, I don’t see why
> it’s necessary. It’s just annoying and gets in the way.
>
>>> Example 3:
>>> public class Outer {
>>> class Inner{
>>> void foo(){
>>> }|<--start position. RET
>>> |<-- expected position
>>> |<-- actual
>>> }
>>> }
>>>
>>> If Inner class has incorrect indentation, subsequent code will also be incorrectly indented.
>>>
>>
>> I would rather inverse the statement, and say that subsequent code is
>> correctly indented, but the preceding code is ignored. As the correct
>> code is
>> ```
>> public class Outer {
>> class Inner{
>> void foo(){
>> }
>>
>> }
>> }
>> ```
>>
>> Indentation at column 8 is expected here, IMO. I'm not sure we should
>> try to work around clearly "incorrectly" indented code.
>
> In my understanding, and in IntelliJ IDEA’s, it should work so that
> it doesn’t matter what syntactic construct, method, or anything else is far
> behind—several levels above, to be exact. Tree-sitter should only care
> about what’s one step behind you. It shouldn’t matter to you that the Inner
> class is misplaced. Your method foo() should focus on where it belongs, even
> if the Inner class has incorrect indentation. This isn’t a hack. It feels
> natural—just follow what’s next to you.
> Right now, it’s just a tightly coupled construct where breaking one
> thing breaks another—or everything altogether.
>
>>
>>>
>>> 4) Java 15 text blocks
>>> Text blocks are not properly handled
>>>
>>> public class TextBlocks {
>>> System.out.println(ctx.fetch("""
>>> SELECT table_schema, count(*)
>>> FROM information_schema.tables
>>> """));
>>> }
>>
>> This isn't valid java is it? I changed it to
>>
>
> Yes, this is invalid code. Again, this touches on how indentation works.
> The machine and the code should think like me:
> "You see I’ve placed triple quotes? You see there’s a method earlier?
> Guess what I’m about to write? Of course, it’s going to be a TEXT BLOCK! Bingo!"
> There’s nothing else I could possibly write there.
>
>> ```
>> public class TextBlocks {
>> public void foo() {
>> System.out.println(ctx.fetch("""
>> SELECT table_schema, count(*)
>> FROM information_schema.tables
>> """));
>>
>> }
>> }
>> ```
>
> Even if I write this code correctly, I still don't have the proper indentation.
> Did you configure anything additional on your end? Out of the box, this doesn't work for me
>
> public class TextBlocks {
> void foo() {
> System.out.println(ctx.fetch(""" <-- 1. press RET
> | <-- and you will be here (Actual)
> | <-- Expected
> """));
> }
> }
>
> java-mode, unlike java-ts-mode, is a bit smarter in this regard - the indentation is at least
> somewhat recognized, but it still ends up in the wrong place. Manual adjustment of rules is required.
>
>> I usually indent stuff like this marking the region then using C-x i
>> then <left> or <right>
> What is C-x i? For me it's insert-file.
>>
>>> - New SQL expressions should be sticky
>> What do you mean here? Align the newlines to the previous line?
>
> I've already forgotten what I meant. But yes, sticky means aligning the new line according
> to the previous line. I think I saw this somewhere or maybe
> I'm confusing it with Stream API method chains.
>
>>> It seems such multiline strings also do not work well, for example:
>>> "'The time has come,' the Walrus said,\n" +
>>
>> Not sure I understand what you mean here
>>
>
> Looks like I said too much here. I apologize. Yes, this works correctly.
> For example, such code will make proper indentation
> String sql = "SELECT u.id, u.name, u.email, COUNT(o.id) AS order_count " +
> "FROM users u " +
> "LEFT JOIN orders o ON u.id = o.user_id " +
>
>>> 6) Multiple Parameters in Methods
>>> Example 1
>>> public record StudentRecord(
>>> String firstName,
>>> String lastName,
>>> Long studentId,
>>> String email)
>>
>> What is wrong here?
>
> There's nothing broken in the code above. But in java-ts-mode it looks like this:
>
> public record StudentRecord(
> String a,
> String b, <--Actual (BAD)
> String b <--Expected (Good)
> )
>
>>> Example 2
>>> public String filterData(@RequestParam(required = false) String name,
>>> @RequestParam(required = false) String name,
>>> @RequestParam(required = false) Integer age
>>> )
>>> java-ts-mode fails to handle these cases correctly.
>>>
>>
>> How so?
> Current behavior in java-ts-mode:
> public String filterData(@RequestParam(required = false) String name,
> @RequestParam(required = false) String name, <--Actual
> @RequestParam(required = false) <--Expected
> )
>
> You understand, I can't even press TAB to move this over?
>
>>> Desired Fontification (Out of the Box):
>>> - Annotations (@Annotations)
>>> - Diamond Brackets (<>)
>>> - Constants, Static Variables, Enum Variables should be highlighted with
>>> distinct colors and optionally italic font
>>> - Unused Variables or Classes (Grayed Out)
>>
>> This feels more like opinions rather than errors, but feature requests
>> are always welcome, of course. You could try to add patches for some of
>> these that I can review?
>
> These might be my personal wishes. But didn't some of this work out of the box in java-mode?
> Am I right about this? Why did this disappear in java-ts-mode?
> Actually, I wouldn't worry about this if I could simply press M-x customize and select
> the needed colors. But... For some reason it's not that simple. In general,
> custom color highlighting setup isn't very oriented towards people without Elisp knowledge.
> Doesn't Emacs philosophy assume that many settings should be done this way
> and all of this is then saved in custom.el?
>
>>> - Unused variables, unused classes, etc., highlighted in gray. Not sure if this can be achieved
>>> with Tree-sitter. Anyway, with Flymake + Eglot, it currently works in a somewhat clunky manner.
>>>
>>
>> This isn't possible with tree sitter. How is eglot clunky here? I'm kind
>> of satisfied, at least when the lsp actually marks these.
>>
>>> Example
>>> public class TextBlocks {
>>> enum AnEnum { CONST1, CONST2 } <-- No effect for unused AnEnum.
>>> public static final String HELLO ="HELLO";
>>>
>>> public static void main(String[] args) {
>>> int i = 0; <-- Flymake identifies as unused but looks unpolished.
>>> System.out.println(HELLO);
>>> }
>>> }
>>>
>>
>> This isn't treesit related, but could be a report for flymake
>> maintainers. We need to get the information from the server, though.
>
> You know what I thought about? I thought that since treesitter
> builds a syntax tree of the file, it should be able to know what
> in this tree has no usage and isn't connected to anything.
>
>>> Overall:
>>>
>>> I may have missed some aspects, but as it stands, Emacs is not
>>> comfortable for Java development with these issues.
>>>
>>
>> Can you provide some sort of priority here? Most feel cosmetic, but
>> there are some real bugs here. One issue I also want, but is not quite
>> sure how to fix yet is the inline sql. We could try to do some multi
>> mode sql syntax highlighting here, possibly.
>
> What priority for each problem? For me, everything is priority.
> Maybe for someone who understands Elisp well, some things might be less urgent.
> Very very very many people don't write anything in the bug tracker.
> Because many things can really look like the user just needs to open
> the Emacs Lisp references manual and do it themselves, rather than asking someone to do it for them.
>
> Slightly off-topic....
>
> Actually, I realized long ago that many things in Emacs are made so that users are given
> some sandbox and provided opportunities to play in it themselves and customize it to their taste.
> But there aren't many things that are really polished and designed to just open and use.
> I haven't tried many programming modes. But recently I discovered Racket-mode.
> God, how neat, polished, well-thought-out this mode looks and requires almost no additional setup
> from the user, and tons of documentation is written.
> For Java, which is in the top 3 languages worldwide, there is NOTHING in Emacs.
> More precisely, there was something and it's probably gathering dust somewhere deep in Emacs' depths,
> something like JDEE or CEDET which 1-2 people use. And nobody talks about this now and nowhere
> is it written about. You need to do some research work to understand what was relevant before
> and what wasn't.
> Do you understand that there isn't even a "run main" button?
> This is funny and sad.
>
>> As another data point - I use Emacs for java development as the sole
>> emacs user on my team in a sea of Intellij users, and I don't really
>> share the view that it isn't comfortable for Java development.
>
> That's cool. I want to be like you and be the only Java developer who works in Emacs,
> but reality is quite different for now. There's so much! So much! That needs to be finished.
> Well okay, I'm lying, not that much.
> I think attaching transient.el + hydra.el + a couple dozen functions and this should
> become almost indistinguishable from IDE and maybe even superior in some places.
> But this still needs to be done...
>
>> There is
>> a quirk here and there, sure, but I'm just as productive as everyone
>> else there. For me the biggest issue is the one case you mentioned here
>> with nested blocks inside of a chain of methods.
>>
>>
>> I'm not sure we should consider what Intellij does as a factual
>> source. Though it is for now the canonical java editor, who knows for
>> how long, and it feels time consuming to jump after a moving target.
>
> As soon as there's some interested person who wants to do some refactoring
> of everything that was previously done for JAVA in Emacs
> and rewrites/writes/adapts it to new realities.
> I'll be able to officially declare that Emacs will be the best editor/IDE for Java.
> But for now, that person hasn't been born yet. Or maybe it all exists already,
> but somewhere in personal repositories on GitHub or somewhere else,
> but won't make it into the core of java-ts-mode.
>
> I know you're not the only one who writes Java in Emacs and remains productive.
> But... I expressed my thoughts about the situation earlier.
> And you understand these are thoughts of an ordinary person, not an Emacs hacker.
> If you can do it, it doesn't mean that thousands of others can do exactly the same.
Ok, I attached a patch-set, if you apply this patch-set on top of _latest master_, most of the fixable problems in this report should be fixed/improved. Now, let me reply each item:
1. In the patch I added java-ts-mode-method-chaining-indent-offset,
defaults to 8
2. Set electric-indent-chars to nil so it doesn’t indent the current
line when you press RET.
3. The indentation is fixed once you type a valid statement, this is
because tree-sitter needs something to generate a parse-tree. We
can add some heuristics that gives a more intuitive indentation
when the line is empty, eg, "if prev line is while and current line
is empty, indent one level", etc. But that’s a more complicated
feature and I’ll defer to Theo.
4.
- Triple quote support in electric-pair-mode -> let’s open a separate
bug report for it and have electric-pair-mode maintainer take a
look.
- In general, TAB in Emacs prog modes indent to a fixed point, rather
than just inserting a tab. I added a indent rule such that aligns a
line in a string block to the previous line, for the first line, it indents one level.
5. That’s hard, if not impossible, for tree-sitter to do. When using a
parser for highlight/indentation, you need provide a correct (or
close to correct) source code for it to work right. That’s part of
the trade-off comparing to regexp-based highlighting.
6. For the parameter indentation, I recently added
c-ts-common-baseline-indent-rules that can handle it correctly. So
if we remove the existing indent rule for the parameters and add
c-ts-common-baseline-indent-rules at the end so the indentation
falls back to it, the indentation would look like this:
public class TextBlocks {
public record StudentRecord(String firstName,
String lastName,
Long studentId,
String email) {
}
public String filterData(@RequestParam(required = false) String name,
@RequestParam(required = false) String name,
@RequestParam(required = false) Integer age
)
}
Seems fair? Theo, WDYT?
> - Annotations (@Annotations)
It seems to work fine? What’s the problem that you see?
> - Diamond Brackets (<>)
Same as above, what’s the desired behavior?
> - Constants, Static Variables, Enum Variables should be highlighted with
> distinct colors and optionally italic font
For constants, they aren’t highlighted in constant face because rules
for ‘definition’ and ‘expression’ feature overrides them with
variable-name face. We can fix this by either moving the ‘constant’
face after ‘definition’ and ‘expression’ feature, or remove the
:override flag for ‘definition’ and ‘expression’ feature. Theo, any
suggestions here?
> - Unused Variables or Classes (Grayed Out)
> - Unused variables, unused classes, etc., highlighted in gray. Not
sure if this can be achieved
Tree-sitter can’t do this. So the only option is to use eglot for it.
Finally, some suggestions on communication. As you said on reddit,
you’re not a native English speaker, so I can’t blame you, but some
minor changes to wording can make your message sound a lot kinder :)
For example, short and imperative sentences like "you understand?"
sounds harsh and condescending; OTOH something like "I hope you can
understand/get that..." is a lot better. As a rule of thumb, the less
certain and the longer your expression is, the softer it sounds ;-)
Yuan
[java-ts-mode-updates.patch (application/octet-stream, attachment)]
[Message part 3 (text/plain, inline)]
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.