Re: [elpa] externals/brief 362d629671: Brief mode v5.88.22 release for various enhancements and bug fixes

Re: [elpa] externals/brief 362d629671: Brief mode v5.88.22 release for various enhancements and bug fixes

[Stefan Monnier]

>  (eval-when-compile
> -  (require 'cl-lib)
> +  (if (version< emacs-version "27.0")
> +      (progn
> +        (require 'cl) ;; Package cl is deprecated for Emacs27+
> +        (require 'cl-macs))
> +    (require 'cl-lib))

This looks like a regression resulting from a misunderstanding.
What are you trying to do here?

The rest of the code only uses `cl-lib` functionality, so (require 'cl)
will only be useful because if it internally does (require 'cl-lib).
In Emacs<24.3, this will fail because (require 'cl) will not define the
macros that the rest of the code needs (e.g. `cl-eval-when`, `cl-loop`, ...).


        Stefan

Re: [elpa] externals/brief 362d629671: Brief mode v5.88.22 release for various enhancements and bug fixes

[Stefan Monnier]

[ Resending with a better email address.  ]

>  (eval-when-compile
> -  (require 'cl-lib)
> +  (if (version< emacs-version "27.0")
> +      (progn
> +        (require 'cl) ;; Package cl is deprecated for Emacs27+
> +        (require 'cl-macs))
> +    (require 'cl-lib))

This looks like a regression resulting from a misunderstanding.
What are you trying to do here?

The rest of the code uses only `cl-lib` functionality, so (require 'cl)
will be useful only if it internally does (require 'cl-lib).
In Emacs<24.3, this will fail because (require 'cl) will not define the
macros that the rest of the code needs (e.g. `cl-eval-when`, `cl-loop`, ...).


        Stefan

Re: [elpa] externals/brief 362d629671: Brief mode v5.88.22 release for various enhancements and bug fixes

[路客]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

Thanks for spotting this.  Yes you are right.  My local branch
was very different from the one on ELPA and it did not use
nadvice package.  Recently I merged ELPA version back and the
code segment you highlighted was quite old and I don't even
remember what it's for, probably a transition code during the
period of transition from `xxx' to `cl-xxx' ...

I've fixed it along with an issue found on the Windows version
-- I just found that the Windows version is a convenient tool to
help me test old Emacs versions like Emacs 24.2.
Thanks.




On Mon, 30 Jan 2023 at 11:01, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> [ Resending with a better email address.  ]
>
> >  (eval-when-compile
> > -  (require 'cl-lib)
> > +  (if (version< emacs-version "27.0")
> > +      (progn
> > +        (require 'cl) ;; Package cl is deprecated for Emacs27+
> > +        (require 'cl-macs))
> > +    (require 'cl-lib))
>
> This looks like a regression resulting from a misunderstanding.
> What are you trying to do here?
>
> The rest of the code uses only `cl-lib` functionality, so (require 'cl)
> will be useful only if it internally does (require 'cl-lib).
> In Emacs<24.3, this will fail because (require 'cl) will not define the
> macros that the rest of the code needs (e.g. `cl-eval-when`, `cl-loop`,
> ...).
>
>
>         Stefan
>
>

-- 
Best regards,
Luke Lee

[-- Attachment #2: Type: text/html, Size: 2007 bytes --]

Re: [elpa] externals/brief 362d629671: Brief mode v5.88.22 release for various enhancements and bug fixes

[Stefan Monnier]

> branch: externals/brief
> commit 362d6296717af611be20cb56592d495c01ea7a00
> Author: Luke Lee <luke@gauss>
> Commit: Luke Lee <luke@gauss>

Please try and use a valid email as commit identifier.

> @@ -457,8 +461,9 @@
>  ;; backward compatibility issues.
>  ;;(require 'replace)
>  
> -(defconst brief-version "5.87"
> -  "The version of this Brief emulator.")
> +;;;###autoload
> +(defconst brief-version "5.88.22"
> +  "Current version of this Brief editor mode/emulator.")

Why autoload?

> -           (not (fboundp 'save-mark-and-excursion)))
> +           (not (fboundp #'save-mark-and-excursion)))

The arg to `fboundp` is a symbol not a function (e.g. it can't be
a (lambda ...) for example), so I think #' is ill-advised.
It works fine, in practice, so it's somewhat of a philosophical issue.

> +  (when (and (version<= "27.0" emacs-version)

I understand efficiency is not a major issue here, but

    (<= 27 emacs-major-version)

works just as well :-)

> -  (scroll-bar-mode -1)             ;; small border without scroll bar
> +  (when (and brief-turn-off-scroll-bar-mode
> +             (fboundp #'scroll-bar-mode))

In which circumstance could `scroll-bar-mode` not be defined?

> +    (scroll-bar-mode -1)             ;; small border without scroll bar
> +    (setq-default scroll-bar-mode -1))

According to its doc, the `scroll-bar-mode` variable can take values
`nil`, `left`, and `right`.  What do you intend to get by setting it to -1?


        Stefan

Re: [elpa] externals/brief 362d629671: Brief mode v5.88.22 release for various enhancements and bug fixes

[路客]

[-- Attachment #1: Type: text/plain, Size: 1808 bytes --]

Sorry that I missed this email.  About the `scroll-bar-mode', I was just
trying to disable it.
Is there another way to disable it?
Thanks.


On Mon, 30 Jan 2023 at 10:58, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > branch: externals/brief
> > commit 362d6296717af611be20cb56592d495c01ea7a00
> > Author: Luke Lee <luke@gauss>
> > Commit: Luke Lee <luke@gauss>
>
> Please try and use a valid email as commit identifier.
>
> > @@ -457,8 +461,9 @@
> >  ;; backward compatibility issues.
> >  ;;(require 'replace)
> >
> > -(defconst brief-version "5.87"
> > -  "The version of this Brief emulator.")
> > +;;;###autoload
> > +(defconst brief-version "5.88.22"
> > +  "Current version of this Brief editor mode/emulator.")
>
> Why autoload?
>
> > -           (not (fboundp 'save-mark-and-excursion)))
> > +           (not (fboundp #'save-mark-and-excursion)))
>
> The arg to `fboundp` is a symbol not a function (e.g. it can't be
> a (lambda ...) for example), so I think #' is ill-advised.
> It works fine, in practice, so it's somewhat of a philosophical issue.
>
> > +  (when (and (version<= "27.0" emacs-version)
>
> I understand efficiency is not a major issue here, but
>
>     (<= 27 emacs-major-version)
>
> works just as well :-)
>
> > -  (scroll-bar-mode -1)             ;; small border without scroll bar
> > +  (when (and brief-turn-off-scroll-bar-mode
> > +             (fboundp #'scroll-bar-mode))
>
> In which circumstance could `scroll-bar-mode` not be defined?
>
> > +    (scroll-bar-mode -1)             ;; small border without scroll bar
> > +    (setq-default scroll-bar-mode -1))
>
> According to its doc, the `scroll-bar-mode` variable can take values
> `nil`, `left`, and `right`.  What do you intend to get by setting it to -1?
>
>
>         Stefan
>
>

-- 
Best regards,
Luke Lee

[-- Attachment #2: Type: text/html, Size: 2617 bytes --]

Re: [elpa] externals/brief 362d629671: Brief mode v5.88.22 release for various enhancements and bug fixes

[Stefan Monnier]

>> > -  (scroll-bar-mode -1)             ;; small border without scroll bar
>> > +  (when (and brief-turn-off-scroll-bar-mode
>> > +             (fboundp #'scroll-bar-mode))
>>
>> In which circumstance could `scroll-bar-mode` not be defined?
>>
>> > +    (scroll-bar-mode -1)             ;; small border without scroll bar
>> > +    (setq-default scroll-bar-mode -1))
>>
>> According to its doc, the `scroll-bar-mode` variable can take values
>> `nil`, `left`, and `right`.  What do you intend to get by setting it to -1?
>
> About the `scroll-bar-mode', I was just trying to disable it.
> Is there another way to disable it?

Doesn't (scroll-bar-mode -1) do that already?


        Stefan

Re: [elpa] externals/brief 362d629671: Brief mode v5.88.22 release for various enhancements and bug fixes

[路客]

[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]

> The arg to `fboundp` is a symbol not a function (e.g. it can't
> be a (lambda ...) for example)
That's a good point, modification reverted.

> (<= 27 emacs-major-version)
Nice, modified likewise.

> Doesn't (scroll-bar-mode -1) do that already?
You're right, corrected as well as some others.
Thanks!

On Mon, 30 Jan 2023 at 10:58, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > branch: externals/brief
> > commit 362d6296717af611be20cb56592d495c01ea7a00
> > Author: Luke Lee <luke@gauss>
> > Commit: Luke Lee <luke@gauss>
>
> Please try and use a valid email as commit identifier.
>
> > @@ -457,8 +461,9 @@
> >  ;; backward compatibility issues.
> >  ;;(require 'replace)
> >
> > -(defconst brief-version "5.87"
> > -  "The version of this Brief emulator.")
> > +;;;###autoload
> > +(defconst brief-version "5.88.22"
> > +  "Current version of this Brief editor mode/emulator.")
>
> Why autoload?
>
> > -           (not (fboundp 'save-mark-and-excursion)))
> > +           (not (fboundp #'save-mark-and-excursion)))
>
> The arg to `fboundp` is a symbol not a function (e.g. it can't be
> a (lambda ...) for example), so I think #' is ill-advised.
> It works fine, in practice, so it's somewhat of a philosophical issue.
>
> > +  (when (and (version<= "27.0" emacs-version)
>
> I understand efficiency is not a major issue here, but
>
>     (<= 27 emacs-major-version)
>
> works just as well :-)
>
> > -  (scroll-bar-mode -1)             ;; small border without scroll bar
> > +  (when (and brief-turn-off-scroll-bar-mode
> > +             (fboundp #'scroll-bar-mode))
>
> In which circumstance could `scroll-bar-mode` not be defined?
>
> > +    (scroll-bar-mode -1)             ;; small border without scroll bar
> > +    (setq-default scroll-bar-mode -1))
>
> According to its doc, the `scroll-bar-mode` variable can take values
> `nil`, `left`, and `right`.  What do you intend to get by setting it to -1?
>
>
>         Stefan
>
>

-- 
Best regards,
Luke Lee

[-- Attachment #2: Type: text/html, Size: 2823 bytes --]