all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Arun Isaac <arunisaac@systemreboot.net>
To: Alex Kost <alezost@gmail.com>
Cc: 26802@debbugs.gnu.org
Subject: bug#26802: Single source file emacs packages get a ".el.el" extension
Date: Sat, 13 May 2017 22:06:05 +0530	[thread overview]
Message-ID: <9b375d38.AEAAKIA9bmkAAAAAAAAAAAO9aM4AAAACwQwAAAAAAAW9WABZFzYx@mailjet.com> (raw)
In-Reply-To: <87vap5xhks.fsf@gmail.com>


I've pushed the other uncontroversial patches.

> It would be a linter only for *.el files, is that what you mean?  AFAIK
> we do not have such specific linters, but yeah, why not, I think it's a
> good idea.

No, it will be a linter for all packages, not just emacs packages.

The following is the current linter for checking source file names. I
have a couple of issues with it, in addition to the new linter feature I
am suggesting. Let me explain.

> (define (check-source-file-name package)
>   "Emit a warning if PACKAGE's origin has no meaningful file name."
>   (define (origin-file-name-valid? origin)
>     ;; Return #t if the source file name contains only a version or is #f;
>     ;; indicates that the origin needs a 'file-name' field.

Isn't this logic somehow backward? Should'nt a predicate called
`file-name-valid?' return #t if the file name is valid, and #f
otherwise?  This seems to be doing the opposite of that, returning #f if
the file name is valid, and #t otherwise.

>     (let ((file-name (origin-actual-file-name origin))
>           (version (package-version package)))
>       (and file-name
>            (not (or (string-prefix? version file-name)
>                     ;; Common in many projects is for the filename to start
>                     ;; with a "v" followed by the version,
>                     ;; e.g. "v3.2.0.tar.gz".
>                     (string-prefix? (string-append "v" version) file-name))))))

I think this check can be done by matching against a single regexp like
so:

(string-match (string-append "^v?" version) file-name)

In addition to this logic, we add an extra condition that makes sure the
version is some substring of the source file name, like so:

(string-match version file-name)

With this new check, single source file packages like
emacs-transpose-frame, etc. which did not have file-name fields would
have raised a lint warning.

>   (let ((origin (package-source package)))
>     (unless (or (not origin) (origin-file-name-valid? origin))
>       (emit-warning package
>                     (G_ "the source file name should contain the package name")
>                     'source))))

> P.S.  Could you please keep my email in Cc?  It would be easier to
> follow the discussion for me, thanks.

Sorry, I thought debbugs does some magic to send out mails to everybody
involved in a specific bug. Didn't realize it was only using Cc. Will
keep you in Cc for this and future mails.

  reply	other threads:[~2017-05-13 16:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-06 12:51 bug#26802: Single source file emacs packages get a ".el.el" extension Arun Isaac
2017-05-09 19:38 ` Alex Kost
2017-05-11 19:19   ` Arun Isaac
2017-05-13  8:54     ` Alex Kost
2017-05-13 16:36       ` Arun Isaac [this message]
2017-05-14 17:15         ` Alex Kost
2017-05-15 13:28           ` Arun Isaac
2017-05-16 17:29             ` Alex Kost
2017-05-17 17:04               ` Arun Isaac
2017-05-21  9:03                 ` Alex Kost
2017-05-13  5:23 ` bug#26802: [PATCH 1/3] gnu: emacs-goto-chg: Set source file-name Arun Isaac
2017-05-13  8:55   ` Alex Kost
2017-05-17 16:52 ` bug#26802: [PATCH 1/4] gnu: lint: Fix typo Arun Isaac
2017-05-18 11:28   ` Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9b375d38.AEAAKIA9bmkAAAAAAAAAAAO9aM4AAAACwQwAAAAAAAW9WABZFzYx@mailjet.com \
    --to=arunisaac@systemreboot.net \
    --cc=26802@debbugs.gnu.org \
    --cc=alezost@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.