Skip to content
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.

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

Fix fzf/position-bottom defcustom type declaration bug & fix byte compiler warnings #83

Merged
merged 6 commits into from
Jan 4, 2023
Merged

Fix fzf/position-bottom defcustom type declaration bug & fix byte compiler warnings #83

merged 6 commits into from
Jan 4, 2023

Conversation

pierre-rouleau
Copy link
Contributor

Which prevented its customization to work properly.
There is no 'bool type. It's 'boolean.

@pierre-rouleau
Copy link
Contributor Author

The fixed reference to free variable is what it says it is: just a fix to prevent the warning. However this code looks suspicious and should probably be changed.

@pierre-rouleau
Copy link
Contributor Author

@bling, you may want to take a look at lispy to help you write lisp code. Trailing closing parens in separate lines in lisp code is not the style you should use.

The setq-local macro accepts only one pair of parameters until it was changed
in Emacs 27 to support multiple pairs.  Since the file states it requires emacs 24.4,
then the code must use the form that was available then and which is still valid.
There is no performance issue in this code to justify using the newer form.
- Fix the Unused lexical variable ‘err’ warning.
- Provide an error message that has more info.
- Cleanup lisp syntax (just for that function): no closing parens on lines
@bling
Copy link
Owner

bling commented Jan 4, 2023

@bling, you may want to take a look at lispy to help you write lisp code. Trailing closing parens in separate lines in lisp code is not the style you should use.

this is due to multiple contributions from different people. if you want the entire file to be consistent, please open a separate PR independent of the defcustom fix. thanks.

@pierre-rouleau
Copy link
Contributor Author

@bling, I will first fix all byte compile warnings. I started with the defcustom error and then fixed several issues in that stream of pull request. There are some warnings that remain and I'll fix them as soon as I get some time.

fzf.el:169:1:Warning: Unused lexical variable ‘term-exec-hook’

In fzf/start:
fzf.el:199:32:Warning: assignment to free variable
    ‘term-suppress-hard-newline’

In fzf/action-find-file-with-line:
fzf.el:217:18:Warning: ‘goto-line’ is for interactive use only; use
    ‘forward-line’ instead.

In end of data:
fzf.el:453:1:Warning: the following functions are not known to be defined: turn-off-evil-mode,
    term-char-mode, projectile-project-root
@pierre-rouleau pierre-rouleau changed the title Fix fzf/position-bottom defcustom type declaration Fix fzf/position-bottom defcustom type declaration & fix byte compiler warnings Jan 4, 2023
@pierre-rouleau
Copy link
Contributor Author

@bling, I fixed all byte compiler warnings as detected by Emacs 26.3 . I'll check with Emacs 27 and 28 soon.

@pierre-rouleau pierre-rouleau changed the title Fix fzf/position-bottom defcustom type declaration & fix byte compiler warnings Fix fzf/position-bottom defcustom type declaration bug & fix byte compiler warnings Jan 4, 2023
@pierre-rouleau
Copy link
Contributor Author

@bling, would it be possible to merge in these PRs? I have found other warnings I'd like to push. I have also found a scenario where the lines returned by fzf includes trailing spaces which cause the use in Emacs to fail. I have fix for those as well. I will report this second issue as a bug and would like to provide a fix for that. Thanks!

@pierre-rouleau
Copy link
Contributor Author

Thanks @bling !

@pierre-rouleau pierre-rouleau deleted the with-warnings-fixed branch January 4, 2023 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants