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

Feature/auth source pass #1

Closed
wants to merge 11 commits into from
Closed

Conversation

DamienCassou
Copy link
Owner

No description provided.

@tzz
Copy link
Collaborator

tzz commented Feb 5, 2017

The changes all look good to me. We'll keep our work here until @DamienCassou gets write access.

Regarding the documentation change f6e4aa8 it looks good.

Regarding the question about why we don't call auth-source-backend-parser-functions a hook var: unfortunately it's not exactly a hook setup, so I couldn't use run-hooks. We want to quit on the first function that returns a parsed backend, while hooks are simply run in sequence. In addition, hook order generally doesn't matter, while here it matters. I hope you agree, and if you see a better way we can change it.

@DamienCassou
Copy link
Owner Author

@tzz what else needs to be done to keep going?

@tzz
Copy link
Collaborator

tzz commented Feb 15, 2017

I was waiting for the copyright assignments and for your write access to the Emacs repo.

If you want to proceed without those, I'll push this into the Emacs repo branch I created.

I'll definitely squash at the end (before merging to Emacs), but if you want to squash now before I merge, go ahead.

@DamienCassou
Copy link
Owner Author

I was waiting for the copyright assignments

I've sent the updated one a few days ago.

and for your write access to the Emacs repo.

I still don't have it but we can carry on.

If you want to proceed without those, I'll push this into the Emacs repo branch I created.

Great, thanks.

I'll definitely squash at the end (before merging to Emacs), but if you want to squash now before I merge, go ahead

I squashed the commits related to auth-source-pass.

DamienCassou and others added 2 commits February 16, 2017 09:36
* lisp/auth-source-pass.el: auth-source backend for password-store.
* test/lisp/auth-source-pass-tests.el: Tests for auth-source-pass
  behavior.
@DamienCassou
Copy link
Owner Author

I've just:

  • rewrote the commit introducing auth-source-pass to always use the hook:
    -(if (boundp 'auth-source-backend-parser-functions)
    -    (add-hook 'auth-source-backend-parser-functions
    -              #'auth-source-pass-backend-parse)
    -  (advice-add 'auth-source-backend-parse :before-until #'auth-source-pass-backend-parse))
    +(add-hook 'auth-source-backend-parser-functions #'auth-source-pass-backend-parse)
  • added a commit for @foudfou's patch to auth-password-store. It was not included before because I had no idea about his copyright assignment form (https://github.com/DamienCassou/auth-password-store/issues/25). I didn't squash the commit into mine to keep @foudfou's name in the git history.

@DamienCassou
Copy link
Owner Author

What is the status of this?

@tzz
Copy link
Collaborator

tzz commented Mar 27, 2017

Sorry for the long silence. I think this is good to merge. I'll ask for comments on emacs-devel and wait a day.

@tzz
Copy link
Collaborator

tzz commented Mar 27, 2017

@DamienCassou @foudfou can you please consider writing docs? If not I'll do it.

@tzz
Copy link
Collaborator

tzz commented Mar 27, 2017

Oh, one more thing: I have to rewrite the commit messages to fit the Emacs requirements. I'll do that when I rebase the commits.

@foudfou can you give me your preferred name and e-mail address? As you have it in the current commit message it's a bit weird and it needs to match up with your copyright assignment.

@foudfou
Copy link

foudfou commented Mar 28, 2017

@tzz these are my preferred name and e-mail address. There's already a tiny patch with this ID (97abf92), and licensing services should be able to match this address if needed.

@DamienCassou
Copy link
Owner Author

@tzz: do you need anything beyond what is in the README? https://github.com/DamienCassou/auth-password-store/blob/master/README.md.

@tzz
Copy link
Collaborator

tzz commented Mar 29, 2017

@DamienCassou I was hoping you'd update the manual (auth-source.texi) but I can do it, as I mentioned.

Noam Postavsky had some small code comments in emacs-devel: https://lists.gnu.org/archive/html/emacs-devel/2017-03/msg00788.html

Do you want to reply on the mailing list or here?

@DamienCassou
Copy link
Owner Author

@DamienCassou I was hoping you'd update the manual (auth-source.texi) but I can do it, as I mentioned.

I understand that. My question was more: do you want me to migrate what's in the README only or is there more you'd like me to write?

Noam Postavsky had some small code comments in emacs-devel: https://lists.gnu.org/archive/html/emacs-devel/2017-03/msg00788.html

I will take care of them.

Do you want to reply on the mailing list or here?

I will fix this PR.

@tzz
Copy link
Collaborator

tzz commented Apr 3, 2017

@DamienCassou if you can take the README and add that to the auth-source manual, I would be very grateful. I don't think more is needed. Thank you for your kind help.

@DamienCassou
Copy link
Owner Author

@tzz I took care of all feedback and enriched the Auth info node. I did the commits in a way you can check all fixes I applied. Feel free to squash everything you need. Also, I know nothing about the texi format: please double check everything.

Thanks


Here are configurations depending on your answers:

@multitable {111} {222} {333} {configuration configuration configuration}
@item @b{1} @tab @b{2} @tab @b{3} @tab Configuration
@item Yes @tab Yes @tab Yes @tab Set up gpg-agent.
@item Yes @tab Yes @tab No @tab You can't, without gpg-agent.
@item Yes @tab Yes @tab No @tab You can't, without gpg-agenz.
Copy link

Choose a reason for hiding this comment

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

typo

@DamienCassou
Copy link
Owner Author

DamienCassou commented Apr 6, 2017 via email

@npostavs
Copy link

npostavs commented Apr 6, 2017

Looks good to me.

DamienCassou pushed a commit that referenced this pull request Apr 9, 2017
The recent changes to src/casefiddle.c cause build failure as seen
below:

    Starting program: /home/npostavs/src/emacs/emacs-bootstrapping/src/temacs
	--batch --load loadup bootstrap
    [Thread debugging using libthread_db enabled]
    Using host libthread_db library "/usr/lib/libthread_db.so.1".
    Loading loadup.el (source)...
    Using load-path (/home/npostavs/src/emacs/emacs-bootstrapping/lisp
	/home/npostavs/src/emacs/emacs-bootstrapping/lisp/emacs-lisp
	/home/npostavs/src/emacs/emacs-bootstrapping/lisp/language
	/home/npostavs/src/emacs/emacs-bootstrapping/lisp/international
	/home/npostavs/src/emacs/emacs-bootstrapping/lisp/textmodes
	/home/npostavs/src/emacs/emacs-bootstrapping/lisp/vc)
    Loading emacs-lisp/byte-run (source)...
    Loading emacs-lisp/backquote (source)...
    Loading subr (source)...
    Loading version (source)...
    Loading widget (source)...
    Loading custom (source)...
    Loading emacs-lisp/map-ynp (source)...
    Loading international/mule (source)...
    Loading international/mule-conf (source)...

    lread.c:3914: Emacs fatal error: assertion failed: !NILP (Vpurify_flag)

    Breakpoint 1, terminate_due_to_signal at emacs.c:363
    363	  signal (sig, SIG_DFL);
    (gdb) bt
    #0  0x0000000000579826 in terminate_due_to_signal at emacs.c:363
    #1  0x000000000060ec33 in die at alloc.c:7352
    emacs-mirror#2  0x000000000066db40 in intern_c_string_1 at lread.c:3914
    emacs-mirror#3  0x0000000000576884 in intern_c_string at lisp.h:3790
    emacs-mirror#4  0x00000000005dc84f in prepare_casing_context at casefiddle.c:69
    emacs-mirror#5  0x00000000005dd37f in casify_object at casefiddle.c:311
    emacs-mirror#6  0x00000000005dd47f in Fcapitalize at casefiddle.c:356
    emacs-mirror#7  0x00000000006325ac in eval_sub at eval.c:2219
    emacs-mirror#8  0x0000000000632368 in eval_sub at eval.c:2184
    emacs-mirror#9  0x000000000063446c in apply_lambda at eval.c:2875
    emacs-mirror#10 0x00000000006329af in eval_sub at eval.c:2294
    emacs-mirror#11 0x000000000062d462 in Fprogn at eval.c:449
    emacs-mirror#12 0x000000000062d4cf in prog_ignore at eval.c:461
    emacs-mirror#13 0x000000000062f19c in Fwhile at eval.c:982
    emacs-mirror#14 0x00000000006321f4 in eval_sub at eval.c:2172
    emacs-mirror#15 0x000000000062d462 in Fprogn at eval.c:449
    emacs-mirror#16 0x000000000062f0c4 in Flet at eval.c:963
    emacs-mirror#17 0x00000000006321f4 in eval_sub at eval.c:2172
    emacs-mirror#18 0x0000000000632963 in eval_sub at eval.c:2290
    emacs-mirror#19 0x000000000062d462 in Fprogn at eval.c:449
    emacs-mirror#20 0x000000000062f0c4 in Flet at eval.c:963
    emacs-mirror#21 0x00000000006321f4 in eval_sub at eval.c:2172
    emacs-mirror#22 0x0000000000668caa in readevalloop at lread.c:1927
    emacs-mirror#23 0x0000000000667253 in Fload at lread.c:1332
    emacs-mirror#24 0x0000000000632683 in eval_sub at eval.c:2233
    emacs-mirror#25 0x0000000000668caa in readevalloop at lread.c:1927
    emacs-mirror#26 0x0000000000667253 in Fload at lread.c:1332
    emacs-mirror#27 0x0000000000632683 in eval_sub at eval.c:2233
    emacs-mirror#28 0x0000000000631be5 in Feval at eval.c:2041
    emacs-mirror#29 0x000000000057e1af in top_level_2 at keyboard.c:1121
    emacs-mirror#30 0x000000000062ffc7 in internal_condition_case at eval.c:1324
    emacs-mirror#31 0x000000000057e1f0 in top_level_1 at keyboard.c:1129
    emacs-mirror#32 0x000000000062f51e in internal_catch at eval.c:1091
    emacs-mirror#33 0x000000000057e0ea in command_loop at keyboard.c:1090
    emacs-mirror#34 0x000000000057d6d5 in recursive_edit_1 at keyboard.c:697
    emacs-mirror#35 0x000000000057d8b4 in Frecursive_edit at keyboard.c:768
    emacs-mirror#36 0x000000000057b55b in main at emacs.c:1687

    Lisp Backtrace:
    "capitalize" (0xffffcf70)
    "format" (0xffffd130)
    "define-charset" (0xffffd370)
    "while" (0xffffd560)
    "let" (0xffffd7c0)
    "dolist" (0xffffd910)
    "let" (0xffffdb70)
    "load" (0xffffdfe0)
    "load" (0xffffe4a0)

* src/casefiddle.c (syms_of_casefiddle): Declare four new symbols:
Qtitlecase, Qspecial_uppercase, Qspecial_lowercase and
Qspecial_titlecase.
(prepare_casing_context): Use aforementioned symbols.
@tzz
Copy link
Collaborator

tzz commented Apr 25, 2017

@DamienCassou sorry for the delay, had a crazy couple of weeks. I'll work on this ASAP.

@tzz
Copy link
Collaborator

tzz commented Apr 26, 2017

@DamienCassou @foudfou I pushed the changes to the emacs.git repo in feature/auth-source-pass. I rebased, squashed, and reformatted your commits to fit the Emacs conventions. You can reset this PR to what I published, if you want to discuss it here. Either way, let me know if you have any comments. I'll plan to merge in a day or two. Thank you for your help.

@tzz
Copy link
Collaborator

tzz commented Apr 27, 2017

This is now merged into emacs.git (rebased and squashed) so this PR can be closed, I think.

@DamienCassou
Copy link
Owner Author

Thank you very much @tzz.

@DamienCassou DamienCassou deleted the feature/auth-source-pass branch May 2, 2017 07:07
DamienCassou pushed a commit that referenced this pull request Jul 30, 2017
* lisp/ses.el (ses-sym-rowcol):  Check that the renamed cell
hashmap has been instantiated before getting data from it.  When
editing several spreadsheets, and you have spreadsheet #1 with a
cell named `foo', and no renamed cell in spreadsheet emacs-mirror#2, then if
you make a formula with `foo' in spreadsheet emacs-mirror#2, not doing this
check will make an error.
(ses-cell-set-formula): Robustify versus incorrect cell references
given in the user provided formula.  An explicit error message is
provided after the action when the user gives an incorrect cell
reference, but the formula edition is not changed.  This means that
if the incorrect reference is to a cell that is created someday,
then this new cell will not have the edited cell in its reference
list.  Fixing this can still be done by editing again the first
cell formula.
(ses-relocate-symbol): Do not create symbol of referred-to cell
when this is a renamed cell.
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.

4 participants