Skip to content

[emacs] Fix Emacs library formatting #76110

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 10, 2024
Merged

[emacs] Fix Emacs library formatting #76110

merged 1 commit into from
Jan 10, 2024

Conversation

darkfeline
Copy link
Contributor

This makes it easier to ship/install these using the builtin Emacs package format (in particular, a Version is required).

@github-actions GitHub Actions
Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang-format mlir labels Dec 20, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 20, 2023 •
edited
Loading

@llvm/pr-subscribers-clang-format

@llvm/pr-subscribers-clang

Author: None (darkfeline)

Changes

This makes it easier to ship/install these using the builtin Emacs package format (in particular, a Version is required).

Full diff: https://github.com/llvm/llvm-project/pull/76110.diff

5 Files Affected:

  • (modified) clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el (+1)
  • (modified) clang/tools/clang-format/clang-format.el (+1)
  • (modified) clang/tools/clang-rename/clang-rename.el (+1)
  • (modified) mlir/utils/emacs/mlir-lsp-client.el (+2)
  • (modified) mlir/utils/emacs/mlir-mode.el (+2-1)
diff --git a/clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el b/clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
index 272f282c47f5fc..f3a949f8c1b552 100644
--- a/clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
+++ b/clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
@@ -1,5 +1,6 @@
 ;;; clang-include-fixer.el --- Emacs integration of the clang include fixer  -*- lexical-binding: t; -*-
 
+;; Version: 0.1.0
 ;; Keywords: tools, c
 ;; Package-Requires: ((cl-lib "0.5") (json "1.2") (let-alist "1.0.4"))
 
diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el
index 30ac7501afcb61..f43bf063c62970 100644
--- a/clang/tools/clang-format/clang-format.el
+++ b/clang/tools/clang-format/clang-format.el
@@ -1,5 +1,6 @@
 ;;; clang-format.el --- Format code using clang-format  -*- lexical-binding: t; -*-
 
+;; Version: 0.1.0
 ;; Keywords: tools, c
 ;; Package-Requires: ((cl-lib "0.3"))
 ;; SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
diff --git a/clang/tools/clang-rename/clang-rename.el b/clang/tools/clang-rename/clang-rename.el
index b6c3ed4c686b76..3f47c11e2c752e 100644
--- a/clang/tools/clang-rename/clang-rename.el
+++ b/clang/tools/clang-rename/clang-rename.el
@@ -1,5 +1,6 @@
 ;;; clang-rename.el --- Renames every occurrence of a symbol found at <offset>.  -*- lexical-binding: t; -*-
 
+;; Version: 0.1.0
 ;; Keywords: tools, c
 
 ;;; Commentary:
diff --git a/mlir/utils/emacs/mlir-lsp-client.el b/mlir/utils/emacs/mlir-lsp-client.el
index 09dfa835deccdc..4397a55e7206ac 100644
--- a/mlir/utils/emacs/mlir-lsp-client.el
+++ b/mlir/utils/emacs/mlir-lsp-client.el
@@ -14,6 +14,8 @@
 ;; See the License for the specific language governing permissions and
 ;; limitations under the License.
 
+;; Version: 0.1.0
+
 ;;; Commentary:
 
 ;; LSP clinet to use with `mlir-mode' that uses `mlir-lsp-server' or any
diff --git a/mlir/utils/emacs/mlir-mode.el b/mlir/utils/emacs/mlir-mode.el
index 69056ba3620eb8..e5947df03f86c8 100644
--- a/mlir/utils/emacs/mlir-mode.el
+++ b/mlir/utils/emacs/mlir-mode.el
@@ -14,6 +14,8 @@
 ;; See the License for the specific language governing permissions and
 ;; limitations under the License.
 
+;; Version: 0.1.0
+
 ;;; Commentary:
 
 ;; Major mode for editing MLIR files.
@@ -96,5 +98,4 @@
 (add-to-list 'auto-mode-alist (cons "\\.mlirbc\\'" 'mlir-mode))
 
 (provide 'mlir-mode)
-
 ;;; mlir-mode.el ends here

@llvmbot
Copy link
Member

llvmbot commented Dec 20, 2023

@llvm/pr-subscribers-mlir

Author: None (darkfeline)

Changes

This makes it easier to ship/install these using the builtin Emacs package format (in particular, a Version is required).

Full diff: https://github.com/llvm/llvm-project/pull/76110.diff

5 Files Affected:

  • (modified) clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el (+1)
  • (modified) clang/tools/clang-format/clang-format.el (+1)
  • (modified) clang/tools/clang-rename/clang-rename.el (+1)
  • (modified) mlir/utils/emacs/mlir-lsp-client.el (+2)
  • (modified) mlir/utils/emacs/mlir-mode.el (+2-1)
diff --git a/clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el b/clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
index 272f282c47f5fc..f3a949f8c1b552 100644
--- a/clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
+++ b/clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
@@ -1,5 +1,6 @@
 ;;; clang-include-fixer.el --- Emacs integration of the clang include fixer  -*- lexical-binding: t; -*-
 
+;; Version: 0.1.0
 ;; Keywords: tools, c
 ;; Package-Requires: ((cl-lib "0.5") (json "1.2") (let-alist "1.0.4"))
 
diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el
index 30ac7501afcb61..f43bf063c62970 100644
--- a/clang/tools/clang-format/clang-format.el
+++ b/clang/tools/clang-format/clang-format.el
@@ -1,5 +1,6 @@
 ;;; clang-format.el --- Format code using clang-format  -*- lexical-binding: t; -*-
 
+;; Version: 0.1.0
 ;; Keywords: tools, c
 ;; Package-Requires: ((cl-lib "0.3"))
 ;; SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
diff --git a/clang/tools/clang-rename/clang-rename.el b/clang/tools/clang-rename/clang-rename.el
index b6c3ed4c686b76..3f47c11e2c752e 100644
--- a/clang/tools/clang-rename/clang-rename.el
+++ b/clang/tools/clang-rename/clang-rename.el
@@ -1,5 +1,6 @@
 ;;; clang-rename.el --- Renames every occurrence of a symbol found at <offset>.  -*- lexical-binding: t; -*-
 
+;; Version: 0.1.0
 ;; Keywords: tools, c
 
 ;;; Commentary:
diff --git a/mlir/utils/emacs/mlir-lsp-client.el b/mlir/utils/emacs/mlir-lsp-client.el
index 09dfa835deccdc..4397a55e7206ac 100644
--- a/mlir/utils/emacs/mlir-lsp-client.el
+++ b/mlir/utils/emacs/mlir-lsp-client.el
@@ -14,6 +14,8 @@
 ;; See the License for the specific language governing permissions and
 ;; limitations under the License.
 
+;; Version: 0.1.0
+
 ;;; Commentary:
 
 ;; LSP clinet to use with `mlir-mode' that uses `mlir-lsp-server' or any
diff --git a/mlir/utils/emacs/mlir-mode.el b/mlir/utils/emacs/mlir-mode.el
index 69056ba3620eb8..e5947df03f86c8 100644
--- a/mlir/utils/emacs/mlir-mode.el
+++ b/mlir/utils/emacs/mlir-mode.el
@@ -14,6 +14,8 @@
 ;; See the License for the specific language governing permissions and
 ;; limitations under the License.
 
+;; Version: 0.1.0
+
 ;;; Commentary:
 
 ;; Major mode for editing MLIR files.
@@ -96,5 +98,4 @@
 (add-to-list 'auto-mode-alist (cons "\\.mlirbc\\'" 'mlir-mode))
 
 (provide 'mlir-mode)
-
 ;;; mlir-mode.el ends here

@darkfeline
Fix Emacs library formatting
e6e31f5 
This makes it easier to ship/install these using the builtin Emacs
package format (in particular, a Version is required).
@darkfeline
Copy link
Contributor Author

Friendly ping, this is a comment change that will make it easier to package these Emacs configs for developers.

@owenca
Copy link
Contributor

owenca commented Jan 6, 2024

@darkfeline I'm ok with adding the Version comment in clang/tools/clang-format/clang-format.el, but you may have to explicitly request others to review the rest of the patch.

@darkfeline
Copy link
Contributor Author

@d0k @r4nt @kirillbobyrev @jpienaar tagging people based on git blame on the files, apologies in advance if you aren't the right persons for this.

d0k
d0k approved these changes Jan 10, 2024
View reviewed changes
Copy link
Member

@d0k d0k left a comment

Choose a reason for hiding this comment

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

I don't know anything about Emacs, but if the version field is required I see no harm in adding it.

@d0k d0k merged commit 5b4abae into llvm:main Jan 10, 2024
@kirillbobyrev
Copy link
Member

Thanks!

As a side note: I did work on clang/tools/clang-rename/clang-rename.el in the past, but clang-rename isn't used much these days and, arguably, is superseded by clangd + LSP in most cases. I don't know if anyone still uses clang-rename.el (I know there was a couple of users within Google, but I'm pretty sure most switched to LSP), so I would say it's OK (and probably desirable) to delete that file.

@darkfeline please let me know if you actually use the clang-rename integration.

@darkfeline
Copy link
Contributor Author

I will ask around internally and let you know after ~2 weeks.

@darkfeline
Copy link
Contributor Author

@kirillbobyrev I have gotten no responses from anyone that uses clang-rename.el or cares about its deletion, take that as you will.

justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
@darkfeline @justinfargnoli
[emacs] Fix Emacs library formatting (llvm#76110)
5b68cc8 
This makes it easier to ship/install these using the builtin Emacs
package format (in particular, a Version is required).
@darkfeline darkfeline deleted the fix branch August 27, 2024 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@d0k d0k d0k approved these changes

Assignees
No one assigned
Labels
clang Clang issues not falling into any other category clang-format mlir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@darkfeline @llvmbot @owenca @kirillbobyrev @d0k