1. Patchew
  2. linux
  3. [PATCH v2 0/2] Makefile: rust-analyzer better error handling, documentation
  4. View patch

[PATCH v2 2/2] Makefile: improve comment documentation for the rust-analyzer target

John Hubbard posted 2 patches 1 year, 5 months ago
There is a newer version of this series
[PATCH v2 2/2] Makefile: improve comment documentation for the rust-analyzer target
Posted by John Hubbard 1 year, 5 months ago 
Replace the cryptic phrase ("IDE support targets") that initially
appears to be about how to support old hard drives, with a few sentences
that explain what "make rust-analyzer" provides.

Cc: Alice Ryhl <aliceryhl@google.com>
Cc: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 07308277a6f5..d22491184af6 100644
--- a/Makefile
+++ b/Makefile
@@ -1967,7 +1967,9 @@ quiet_cmd_tags = GEN     $@
 tags TAGS cscope gtags: FORCE
 	$(call cmd,tags)
 
-# IDE support targets
+# Generate rust-project.json, which does for Rust what clangd's
+# compile_commands.json does for C/C++: provides a symbol browsing database for
+# code editors and IDEs (Integrated Development Environments).
 PHONY += rust-analyzer
 rust-analyzer:
 	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh
-- 
2.45.2
Re: [PATCH v2 2/2] Makefile: improve comment documentation for the rust-analyzer target
Posted by Alice Ryhl 1 year, 5 months ago 
On Thu, Jun 20, 2024 at 10:55 PM John Hubbard <jhubbard@nvidia.com> wrote:
>
> Replace the cryptic phrase ("IDE support targets") that initially
> appears to be about how to support old hard drives, with a few sentences
> that explain what "make rust-analyzer" provides.
>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 07308277a6f5..d22491184af6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1967,7 +1967,9 @@ quiet_cmd_tags = GEN     $@
>  tags TAGS cscope gtags: FORCE
>         $(call cmd,tags)
>
> -# IDE support targets
> +# Generate rust-project.json, which does for Rust what clangd's
> +# compile_commands.json does for C/C++: provides a symbol browsing database for
> +# code editors and IDEs (Integrated Development Environments).

Is "symbol browsing database" the right word here? It's not actually a
list of symbols, but instructions for how to compile the code.

Alice
Re: [PATCH v2 2/2] Makefile: improve comment documentation for the rust-analyzer target
Posted by Miguel Ojeda 1 year, 5 months ago 
On Wed, Jun 26, 2024 at 10:08 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Is "symbol browsing database" the right word here? It's not actually a
> list of symbols, but instructions for how to compile the code.

Yeah, I think the sentence mixes a bit what the file is with what
(some of) the users of the file do with it.

What about something like (getting inspiration from the official documentation):

    # Generate `rust-project.json` (a file that describes the
structure of non-Cargo Rust projects) for `rust-analyzer` (an
implementation of the Language Server Protocol).

I would avoid mentioning `compile_commands.json`, since they are
slightly different the Rust one does not really contain the compile
commands.

As for "IDE", I am happy either way (i.e. removing it or not). Another
alternative that may clarify by giving context could be "Editor / IDE"
(since one may use LSP with "simple editors" and not "full IDEs"
anyway).

With that changed, if Masahiro wants to pick these two up:

    Acked-by: Miguel Ojeda <ojeda@kernel.org>

Otherwise I am happy to take them too.

Cheers,
Miguel
Re: [PATCH v2 2/2] Makefile: improve comment documentation for the rust-analyzer target
Posted by John Hubbard 1 year, 5 months ago 
On 6/26/24 1:42 AM, Miguel Ojeda wrote:
> On Wed, Jun 26, 2024 at 10:08 AM Alice Ryhl <aliceryhl@google.com> wrote:
>>
>> Is "symbol browsing database" the right word here? It's not actually a
>> list of symbols, but instructions for how to compile the code.
> 
> Yeah, I think the sentence mixes a bit what the file is with what
> (some of) the users of the file do with it.
> 
> What about something like (getting inspiration from the official documentation):
> 
>      # Generate `rust-project.json` (a file that describes the
> structure of non-Cargo Rust projects) for `rust-analyzer` (an
> implementation of the Language Server Protocol).
> 
> I would avoid mentioning `compile_commands.json`, since they are
> slightly different the Rust one does not really contain the compile
> commands.
> 
> As for "IDE", I am happy either way (i.e. removing it or not). Another
> alternative that may clarify by giving context could be "Editor / IDE"
> (since one may use LSP with "simple editors" and not "full IDEs"
> anyway).
> 

OK, with those changes applied (minus the backticks, which don't want to
be in this particular Makefile), we have:

Author: John Hubbard <jhubbard@nvidia.com>
Date:   Thu Jun 20 13:54:53 2024 -0700

     Makefile: improve comment documentation for the rust-analyzer target
     
     Replace the cryptic phrase ("IDE support targets") that initially
     appears to be about how to support old hard drives, with a few sentences
     that explain what "make rust-analyzer" provides.
     
     Cc: Alice Ryhl <aliceryhl@google.com>
     Reviewed-by: Finn Behrens <me@kloenk.dev>
     Acked-by: Miguel Ojeda <ojeda@kernel.org>
     Signed-off-by: John Hubbard <jhubbard@nvidia.com>

diff --git a/Makefile b/Makefile
index 204e9be0e010..7db597bdb09d 100644
--- a/Makefile
+++ b/Makefile
@@ -1967,7 +1967,9 @@ quiet_cmd_tags = GEN     $@
  tags TAGS cscope gtags: FORCE
         $(call cmd,tags)
  
-# IDE support targets
+# Generate rust-project.json (a file that describes the structure of non-Cargo
+# Rust projects) for rust-analyzer (an implementation of the Language Server
+# Protocol).
  PHONY += rust-analyzer
  rust-analyzer:
         $(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh


> With that changed, if Masahiro wants to pick these two up:
> 
>      Acked-by: Miguel Ojeda <ojeda@kernel.org>
> 
> Otherwise I am happy to take them too.
> 
> Cheers,
> Miguel

I can send out a v3, I'll wait to see if there is any additional feedback
first, though.


thanks,
-- 
John Hubbard
NVIDIA
Re: [PATCH v2 2/2] Makefile: improve comment documentation for the rust-analyzer target
Posted by Finn Behrens 1 year, 5 months ago 

On 20 Jun 2024, at 22:54, John Hubbard wrote:

> Replace the cryptic phrase ("IDE support targets") that initially
> appears to be about how to support old hard drives, with a few sentences
> that explain what "make rust-analyzer" provides.
>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 07308277a6f5..d22491184af6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1967,7 +1967,9 @@ quiet_cmd_tags = GEN     $@
>  tags TAGS cscope gtags: FORCE
>  	$(call cmd,tags)
>
> -# IDE support targets
> +# Generate rust-project.json, which does for Rust what clangd's
> +# compile_commands.json does for C/C++: provides a symbol browsing database for
> +# code editors and IDEs (Integrated Development Environments).
>  PHONY += rust-analyzer
>  rust-analyzer:
>  	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh
> -- 
> 2.45.2

Reviewed-by: Finn Behrens <me@kloenk.dev>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.