Skip to content

[Fix #313] pf doesn't understand %& #327

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
Apr 12, 2016
Merged

[Fix #313] pf doesn't understand %& #327

merged 1 commit into from
Apr 12, 2016

Conversation

expez
Copy link
Member

@expez expez commented Apr 7, 2016

No tests are added because of a long standing bug in the test framework
where use of %s triggers a 'wrong number of args passed to format'
error.

@expez
Copy link
Member Author

expez commented Apr 7, 2016

The tests pass locally, not sure what the weird stuff on travis is about.

Warning (emacs): Unrecognized key: _r_
Warning (emacs): Unrecognized key: _r_
Warning (emacs): Unrecognized key: _r_
Warning (emacs): Unrecognized key: _r_
Warning (emacs): Unrecognized key: _r_
Warning (emacs): Unrecognized key: _r_
Warning (emacs): Unrecognized key: _r_
Warning (emacs): Unrecognized key: _r_
Warning (emacs): Unrecognized key: _r_
Warning (emacs): Unrecognized key: _r_

Times a million :(

@Malabarba
Copy link
Member

where use of %s triggers a 'wrong number of args passed to format'

Sorry if this was already suggested, but is it possible to use %%s instead?

Malabarba
Malabarba reviewed Apr 7, 2016
View reviewed changes
clj-refactor.el
@@ -2611,10 +2611,12 @@ See: https://github.com/clojure-emacs/clj-refactor.el/wiki/cljr-update-project-d
(cljr--goto-fn-definition)
(let ((fn-start (point))
var replacement)
(while (re-search-forward "%[1-9]?" (cljr--point-after 'paredit-forward) t)
(while (re-search-forward "%[1-9]?&?" (cljr--point-after 'paredit-forward) t)
Copy link
Member

Choose a reason for hiding this comment

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

Or "%[1-9&]?"?

@expez
Copy link
Member Author

expez commented Apr 7, 2016

Sorry if this was already suggested, but is it possible to use %%s instead?

Unfortunately not :(

@expez expez force-pushed the pf-and-varargs branch 2 times, most recently from bd809d2 to b95bfd2 Compare April 7, 2016 14:25
@benedekfazekas
Copy link
Member

I guess you mean this bug there might be a workaround altho ugly. can you pls add the test to the commit or just post a patch file so I can play around a bit with it.

travis failure: weird. everything passes locally for me too.
[edit] rerun 'fixed' the build issue tho both for this and for #326

@expez
Copy link
Member Author

expez commented Apr 9, 2016

There is no workaround, short of coupling our tests to the number of calls to format used in the external test library. I'm not going to count, nor maintain such a fragile test.

Looks like we got a bad container or something, the travis tests are passing again on this branch.

expez
expez reviewed Apr 9, 2016
View reviewed changes
CHANGELOG.md
@@ -12,6 +12,7 @@

### Bugs fixed

- [#313](https://github.com/clojure-emacs/clj-refactor.el/issues/313) `*data-readers*` teach `pf` about function literals using `%&`.
- [#320](https://github.com/clojure-emacs/clj-refactor.el/issues/320) `*data-readers*` ignored when searching for macros.
Copy link
Member Author

Choose a reason for hiding this comment

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

Lol

that copy paste error propagated through all my commits

@benedekfazekas
Copy link
Member

There is no workaround, short of coupling our tests to the number of calls to format used in the external test library. I'm not going to count, nor maintain such a fragile test.

fair enough

@expez
[Fix #313] pf doesn't understand varargs
435515e 
No tests are added because of a long standing bug in the test framework
where use of `%s` triggers a 'wrong number of args passed to format'
error.
@expez expez merged commit cc48705 into master Apr 12, 2016
@expez expez deleted the pf-and-varargs branch April 12, 2016 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@expez @Malabarba @benedekfazekas